Skip to content

Don't index into __builtins__ (not supported by PyPy). #8331

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 20, 2017

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 18, 2017

cf #8330.

@@ -6147,13 +6147,6 @@ def _normalize_input(inp, ename='input'):
if bins is None:
bins = rcParams['hist.bins']

# xrange becomes range after 2to3
bin_range = range
Copy link
Member

Choose a reason for hiding this comment

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

I have a slight preference for

bin_range = range
del range

It causes less code churn and keeps the code below clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds reasonable, fixed

@anntzer anntzer force-pushed the dont-use-__builtins__ branch from dd45300 to 0519ea3 Compare March 18, 2017 22:47
@@ -6147,12 +6147,9 @@ def _normalize_input(inp, ename='input'):
if bins is None:
bins = rcParams['hist.bins']

# xrange becomes range after 2to3
# Avoid shadowing the builtin.
Copy link
Member

Choose a reason for hiding this comment

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

There's a lot of code between the argument definition and the renaming here; I think this really should be the first thing in the function to avoid any and all possible shadowing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds reasonable, fixed.

@anntzer anntzer force-pushed the dont-use-__builtins__ branch from 0519ea3 to 7eee4c2 Compare March 19, 2017 23:14
@QuLogic QuLogic changed the title Don't index into __builtins__ (not supported by PyPy). [MRG+1] Don't index into __builtins__ (not supported by PyPy). Mar 20, 2017
@QuLogic QuLogic added this to the 2.0.1 (next bug fix release) milestone Mar 20, 2017
@NelleV NelleV merged commit 2063e81 into matplotlib:master Mar 20, 2017
@NelleV
Copy link
Member

NelleV commented Mar 20, 2017

Thanks @anntzer !

@anntzer anntzer deleted the dont-use-__builtins__ branch March 20, 2017 14:19
QuLogic pushed a commit to QuLogic/matplotlib that referenced this pull request Mar 27, 2017
Don't index into __builtins__ (not supported by PyPy).
@QuLogic QuLogic changed the title [MRG+1] Don't index into __builtins__ (not supported by PyPy). Don't index into __builtins__ (not supported by PyPy). Mar 27, 2017
@QuLogic
Copy link
Member

QuLogic commented Mar 27, 2017

Backported to v2.0.x as 64b9ece.

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.

4 participants