-
-
Notifications
You must be signed in to change notification settings - Fork 8k
DOC: Scale axis parameter #30404
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
DOC: Scale axis parameter #30404
Conversation
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.
A bit streamlined?
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.
My reviews might not be going through, sorry
lib/matplotlib/scale.py
Outdated
object as first argument. We plan to remove it in the future, because | ||
we want to make a scale object usable by multiple | ||
`~matplotlib.axis.Axis`\es at the same time. | ||
|
||
The current recommendation for `.ScaleBase` subclasses is to have the | ||
*axis* parameter for API compatibility, but not make use of it. |
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.
object as first argument. We plan to remove it in the future, because | |
we want to make a scale object usable by multiple | |
`~matplotlib.axis.Axis`\es at the same time. | |
The current recommendation for `.ScaleBase` subclasses is to have the | |
*axis* parameter for API compatibility, but not make use of it. | |
object as first argument. | |
The current recommendation for `.ScaleBase` subclasses is to have the | |
*axis* parameter for API compatibility, but not make use of it. This is | |
because we plan to remove this argument to make a scale object usable | |
by multiple `~matplotlib.axis.Axis`\es at the same time. | |
Put the status quo and then the plan?
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.
The thing is that the status quo will be changing thoughout the course of deprecation as laid out in #29349. And then I figured it is simpler to have a defined target state and then explain what the current recommendation it. It's simpler to adapt the status quo if it is a dedicated paragraph.
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.
So why not just use a formal pending deprecation?
I think "we plan to do x" isn't something that belongs in API/reference docs b/c the phrasing "plan to" leaves an out of we might not do it and therefore it's not worth it for the user to make this change whereas the deprecation machinery conveys "we will do x" and therefore the user needs to make this change.
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.
The deprecation process is quite involved here. Basically we have to make sure that in the deprecation period our own scales work with and without the axis parameter, and we must support third party scales that may or may not have (and theoretically even use) that parameter. There‘s some more work needed for that.
Only then, people are able to start changing their code. A deprecation is the call to action to do these changes (basically saying „don’t do this anymore, but do this instead“). The pending aspect only defines which target group we address (library authors vs end users)
so yes there will be a deprecation, but that requires between 1-3 further PRs. Hopefully I can get these in for 3.11, but I want to make sure (particularly for myself and reviewers of the upcoming PRs) that we have a consistent view on the current state. I don’t think we need overthink this description.
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.
but I want to make sure (particularly for myself and reviewers of the upcoming PRs) that we have a consistent view on the current state
That can happen in a comment, which is more or less what I think is happening in the font work. The reason I'm being annoying here is a worry about how quickly scope/modality creep happens in the docs.
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.
But also I'm very explicitly not blocking.
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.
Ok, I've took your suggestion. T.b.h. I don't really care too much on the wording and if that helps to get this through, let's do it.
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Co-authored-by: hannah <story645@gmail.com>
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.
Made some grammar/diction comments since this can't be merged w/o the lint fixes, you're welcome to take or leave them before merging.
Co-authored-by: hannah <story645@gmail.com>
Document the current state of of the about-to-be-removed axis parameter of Scales as given by #29349
and #29988.