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

Tidy up the matplotlib.__init__ documentation. #1950

Merged
merged 1 commit into from May 14, 2013

Conversation

pelson
Copy link
Member

@pelson pelson commented Apr 26, 2013

The matplotlib namespace is really cluttered. Many of the functions here could be static methods or instance methods of the RcParams dictionary class.

I don't want to hold up this PR, but if there is appetite to break backwards compatibility, I'd be willing to tidy it up in a follow-on PR.

@mdboom
Copy link
Member

mdboom commented Apr 30, 2013

Looks fine to me. I worry about keeping the manual list in matplotlib_configuration_api.rst up-to-date, but as you say, the only way to really fix that is to remove some things from __init__.py. Maybe we schedule that for post 1.3?

@pelson
Copy link
Member Author

pelson commented May 1, 2013

I worry about keeping the manual list in matplotlib_configuration_api.rst up-to-date

I empathise with that, but at the end of the day, if somebody adds new functionality to the matplotlib/__init__.py you would expect them to check that what they are adding is documented (as you would expect the reviewer to do also). So personally I'm comfortable with enumerating all of the necessary items in this special case.

Maybe we schedule that for post 1.3?

Hmmm. We could start the deprecation process for v1.3 potentially, and then have them removed in v1.4?

d = {}
for root, dirs, files in os.walk(datapath):
for root, _, files in os.walk(datapath):
Copy link
Member

Choose a reason for hiding this comment

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

Just a style opinion, not a demand for change: I much prefer the original; the use of the bare underscore makes this sort of thing less readable and slightly ugly to my eye. In the case of the path.split(), an alternative is to index it with [-1].

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. Thanks @efiring. I only did this because my editor was shouting at me that dirs was unused - the underscores use is not mentioned in PEP8 (though there is an interesting question on http://stackoverflow.com/questions/11486148/unused-variable-naming-in-python).

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the use of underscores: it makes explicit that those variable are not used in this file, which is a convention in python and some other languages (perl?). IMO, we should stick to convention as much as possible.

Also, that makes pyflakes not happy.

@efiring
Copy link
Member

efiring commented May 1, 2013

@pelson, I agree with your original comment about the clutter in init.py. The API is a mess.

pelson added a commit that referenced this pull request May 14, 2013
Tidy up the matplotlib.__init__ documentation.
@pelson pelson merged commit c1a1079 into matplotlib:master May 14, 2013
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.

None yet

4 participants