Skip to content

Validate positional parameters of add_subplot() #16527

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 1 commit into from
Mar 14, 2020

Conversation

timhoffm
Copy link
Member

PR Summary

Closes #16016.

@timhoffm timhoffm added this to the v3.3.0 milestone Feb 16, 2020
@timhoffm timhoffm force-pushed the validate-add_subplot branch 2 times, most recently from 64d00a9 to 328e336 Compare February 16, 2020 15:04
@timhoffm timhoffm force-pushed the validate-add_subplot branch 2 times, most recently from c705c7c to 808ce2b Compare February 16, 2020 21:49
@timhoffm timhoffm force-pushed the validate-add_subplot branch from 808ce2b to f495b7b Compare March 5, 2020 17:36
separately as three single-digit integers, i.e.
``fig.add_subplot(235)`` is the same as
``fig.add_subplot(2, 3, 5)``. Note that this can only be used
if there are no more than 9 subplots.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not technically true, you could pass 999 to get the 9th subplot of 81, but I guess we really don't want to encourage this form too much anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, in both the observation and the conclusion 😄.


if isinstance(args[0], SubplotBase):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type is not documented; should it be?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not documented as type, but in the description:

            In rare circumstances, `.add_subplot` may be called with a single
            argument, a subplot axes instance already created in the
            present figure but not in the figure's list of axes.

I'm not even sure what this is good for or how to achieve that, but that code path triggers on the tests. I've intentionally omitted this form the types. I assume adding it to the types without big warnings would probably do more harm than it's worth, because people will start to put axes from other figures in.

@timhoffm timhoffm force-pushed the validate-add_subplot branch 2 times, most recently from 3aa7bfe to 714d16b Compare March 10, 2020 14:46
@QuLogic
Copy link
Member

QuLogic commented Mar 10, 2020

This is not passing; I think you may need to reset the warning filter (if pytest doesn't do it for you), or else doubled warnings do not get output.

@timhoffm timhoffm force-pushed the validate-add_subplot branch from 714d16b to fa0b9ee Compare March 11, 2020 09:17
@timhoffm
Copy link
Member Author

I was just checking for the wrong warning text.

Copy link
Member

@tacaswell tacaswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'm fine with this going in as is, but holding off in case @timhoffm wants to update the error message. If not, anyone can (self) merge.

@timhoffm
Copy link
Member Author

I will update the message.

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

Successfully merging this pull request may close these issues.

Better argument checking of subplot definition in add_subplot()
3 participants