-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
DOC: Remove Tick object details from artist tutorial #31692
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure how I feel about removing this section. Although I of course agree that we don't want to encourage users to directly modify the ticks, the section could be of interest to someone who just wants to know how it all fits together. There is a thread running through the whole tutorial that everything you see in the plot is ultimately made from the primitive artists, and this maybe helps to complete that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section consists of three parts:
tick_params- including at this side, where we didn't realize this defeats the purpose. While one could revert this, I think this is an indicator that the section should not exists.I'm inclined to say that these are implementation details a user should not need to know. - At least I wish we were in that state and I would want to move in that direction. The whole ticks concept should be handled through a single object, something like a tick collection (likely not a collection in the sense of our Collection class), but some aggregate object as the single public interface. This may internally be implemented via Line2Ds and Texts, or maybe we want a LineCollection, or something else completely.
You could argue that we're not yet there and therefore should explain the constituents. I was hoping that we can just be silent about it because it's aspirationally not relevant. The issue is that the section as is without giving scope and guidance is the worst option. Removing it is better than keeping. I could alternatively restore and add a big warning "for information only, don't use" at the top of the section. I don't have the willingess or time to rewrite the section into something reasonably right, helpful and understandable, in particular given that the overall tutorial has several and more severe shortcomings.