X Tutup
Skip to content

DOC: clarify that secondary_[xy]axis returns an Axes subclass#28953

Closed
rcomer wants to merge 1 commit intomatplotlib:mainfrom
rcomer:doc-secondary-axis
Closed

DOC: clarify that secondary_[xy]axis returns an Axes subclass#28953
rcomer wants to merge 1 commit intomatplotlib:mainfrom
rcomer:doc-secondary-axis

Conversation

@rcomer
Copy link
Copy Markdown
Member

@rcomer rcomer commented Oct 8, 2024

PR summary

I had expected secondary_xaxis to return something that behaves like XAxis, so was surprised to get

AttributeError: 'SecondaryAxis' object has no attribute 'set_tick_params'. Did you mean: 'tick_params'?

PR checklist

@rcomer rcomer added the Documentation: API files in lib/ and doc/api label Oct 8, 2024
@rcomer rcomer changed the title DOC: clarify that secondary_[xy]axis returns an Axes subclass [ci-doc] DOC: clarify that secondary_[xy]axis returns an Axes subclass Oct 8, 2024
@timhoffm
Copy link
Copy Markdown
Member

timhoffm commented Oct 8, 2024

Not necessarily to be done in this PR, but the docstring should be substantially enhanced to explain what this is for and how it works, some points are:

  • you want second x axis, that is related to the x axis, e.g. show angles in degree and rad or show period and frequency
  • this is realized by overlaying a child Axes and hiding all parts but its x axis.
  • often, you do not add any data to the child Axes (and by default that does not make sense because it's y axis is hidden, so one cannot infer what y-values would belong to the data points.)
  • One may contrast this with twin axes that share one axis but explicitly differ in the other axis to be able to show to different types of data overlaid.

Not sure to mention it, but IMHO the implementation as child Axes is a workaround implementation detail. A possibly cleaner, more general approach would be to support multiple axises on one Axes object.

@jklymak
Copy link
Copy Markdown
Member

jklymak commented Oct 8, 2024

Not sure to mention it, but IMHO the implementation as child Axes is a workaround implementation detail. A possibly cleaner, more general approach would be to support multiple axises on one Axes object.

If I recall, the main reason it is an Axes rather than an Axis, is that Axis does not have the concept of an x/ylabel - that is an Axes property. Similarly I think there are numerous tick conveniences on Axes that are less easy on Axis. Not to say one couldn't come up with an API for multiple xlabels etc, but simplest was just a child Axes.

@timhoffm
Copy link
Copy Markdown
Member

While pragmatism tells me to merge this, this feels wrong. While we interpret type fields loosely target towards readability (instead of formal type specifications), the type field is not the place for context information. The minimal solution would be to move that to the description. OTOH, the underlying problem is that SecondaryAxes itself is not documented. It should be, and then the type link would redirect to the documentation.

@rcomer
Copy link
Copy Markdown
Member Author

rcomer commented Oct 21, 2024

Fair enough. I don't really have bandwidth to propose a better change right now, so let's just close this.

@timhoffm
Copy link
Copy Markdown
Member

I've created #29004 as a reminder. Will care for it, when there is time.

@rcomer rcomer deleted the doc-secondary-axis branch October 21, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation: API files in lib/ and doc/api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

X Tutup