Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure most Chart methods link to the appropriate class #2610

Closed
wants to merge 3 commits into from

Conversation

multimeric
Copy link
Contributor

@multimeric multimeric commented Jun 2, 2022

Closes #2608.

This renders like this:

Before:
image

After:
image

Note that these are now hyperlinks.

@joelostblom
Copy link
Contributor

Thank you @multimeric ! This seems like a good way of dealing with the issue you raised. I am going to try it out locally after some of the other doc PRs have been merged (e.g. #2566 )

@multimeric multimeric changed the title Patch the configure_X docstrings to link to the appropriate class Ensure most Chart methods link to the appropriate class Jun 9, 2022
@multimeric
Copy link
Contributor Author

I've now done the same for the transform_* and mark_* methods.

Before:
image
image

After:
image
image

@joelostblom
Copy link
Contributor

Nice! I agree that this addition makes the docs more friendly to use. @jakevdp Should new changes like these changes only be made to the v5 schema or also "backported" to v4 and v3 as in the PR currently?

@mattijn
Copy link
Contributor

mattijn commented Nov 17, 2022

I favor not doing the backport.

@joelostblom
Copy link
Contributor

Me too so let's go with that. @multimeric sorry for the delay here. Could you remove the v3 and v4 versions of mixins.py from this PR? I am also not sure about the changes to API.rst, but maybe that's because we're out of sync with master, could you rebase and double check API.rst manually?

@mattijn
Copy link
Contributor

mattijn commented Feb 19, 2023

@binste, could you please check if this PR is still of interest?

@binste
Copy link
Contributor

binste commented Feb 19, 2023

Changes look great and relevant, I think it's a good improvement to the docs. Could you merge in master and run python tools/generate_schema_wrapper.py again and then ping me? I could then build the docs locally and see if the changes still render fine, which they should.

@mattijn
Copy link
Contributor

mattijn commented Feb 19, 2023

There are currently numerous conflicts that need resolving first. I can have a look later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link to the appropriate "schema wrapper" objects in the API docs
4 participants