Skip to content

Parameterize test_fill_between and test_fill_betweenx #8729

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 2 commits into from
Jun 8, 2017

Conversation

patniharshit
Copy link
Contributor

@patniharshit patniharshit commented Jun 7, 2017

Parametrize test_fill_between and test_fill_betweenx and drops plot call
fixes #8698

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/whats_new.rst if major new feature
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

(np.arange(0.0, 2, 0.02), 3, np.zeros((2, 2)))
]
)
def test_fill_betweenx_input(y, x1, x2):
fig = plt.figure()
ax = fig.add_subplot(211)
Copy link
Member

Choose a reason for hiding this comment

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

Anyone got any idea why this line is here? Looks pointless to me...

Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is the line you meant to comment on? How else would you plot without an Axes?

Copy link
Member

Choose a reason for hiding this comment

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

🤦‍♂️ I thought the previous line was plt.subplots (which I'm far too used too). Still not sure why it's called with 211 instead of 111, but doesn't make a difference either way.

Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

👍 Thanks, happy for this to be merged when tests pass.

(np.arange(0.0, 2, 0.02), np.zeros((2, 2)), 3),
(np.arange(0.0, 2, 0.02), 3, np.zeros((2, 2)))
]
)
Copy link
Member

Choose a reason for hiding this comment

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

Please add some ids to this call; you can use the unique parts of the original functions names, probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. Added.

(np.arange(0.0, 2, 0.02), np.zeros((2, 2)), 3),
(np.arange(0.0, 2, 0.02), 3, np.zeros((2, 2)))
]
)
Copy link
Member

Choose a reason for hiding this comment

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

And some ids here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@QuLogic QuLogic merged commit 34fda3e into matplotlib:master Jun 8, 2017
@QuLogic QuLogic added this to the 2.1 (next point release) milestone Jun 8, 2017
@patniharshit patniharshit deleted the test_fill_between branch June 8, 2017 20:23
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.

fill_between_2d tests are testing the wrong thing
3 participants