Skip to content
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

Improve docstring of Axes.pcolorfast #11363

Merged
merged 1 commit into from Jun 5, 2018

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jun 1, 2018

PR Summary

As part of #10148: Docstring update for Axes.pcolorfast.

@efiring since you seem to be the original author, can you tell me something about the current state. Is the experimental statement still true? If so, is that because of the limitations mentioned in the current docs? Are there any known bugs? Are there any plans to make it a full method? Or is this maybe a dead end? Are users supposed to use this instead of pcolor() or pcolormesh() for the cases that are currently supported? Thanks a lot.

*X* and *Y* are used to specify the coordinates of the
quadilaterals. There are different ways to do this:

- Use tuples ``xr=(xmin, xmax)`` and ``yr=(ymin, ymax)`` to define a
Copy link
Member

Choose a reason for hiding this comment

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

PEP8 is mad about the length of this line...

@efiring
Copy link
Member

efiring commented Jun 3, 2018

pcolorfast has been there for a long time now, and I haven't seen a clamor to remove it. It's an odd method in that it can return any of 3 different objects; but it serves a definite purpose. I believe the contents of the present docstring correctly describe how it works. I would not object to removing the "Experimental" part, but at the same time, there are some things that should be fixed--especially the lack of support for log scales when the intermediate-speed option is invoked. I suspect this would not be terribly hard for someone who understands how the normal image and pcolormesh handle scales, but it's all pretty complicated to try to figure out. Once it is understood, it is probably a matter of adding only a few lines of code to image.PcolorImage.

Also, I think that the 3 objects returned by pcolorfast could be made more similar than they are now.

(Quadmesh (and pcolormesh) desperately needs some refactoring and reworking also.)

@timhoffm
Copy link
Member Author

timhoffm commented Jun 3, 2018

Thanks @efiring

For now, I'd just leave the "Experimental" part. I think it's can be rather confusing to have methods methods (imshow, pcolor, pcolormesh and pcolorfast) that do very similar things. The latter being sort of smart to choose between the others. That should really be explained to the user and it would make sense to try and unify this further as @efiring mentioned. So let's not stabilize the current API further by removing the "Experimental" statement. - At least not as part of this docstring cleanup. That should be a separate and well-thought PR.

@phobson phobson merged commit 8eb2359 into matplotlib:master Jun 5, 2018
@timhoffm timhoffm added this to the v3.0 milestone Jun 5, 2018
@timhoffm timhoffm deleted the doc-axes-pcolorfast branch June 5, 2018 19:31
@timhoffm
Copy link
Member Author

timhoffm commented Jun 5, 2018

@meeseeksdev backport to v2.2.x

@matplotlib matplotlib deleted a comment from lumberbot-app bot Jun 5, 2018
@tacaswell tacaswell modified the milestones: v3.0, v2.2.3 Jun 10, 2018
jklymak added a commit that referenced this pull request Jun 13, 2018
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.

None yet

5 participants