Skip to content

MAINT: Misc numeric cleanup #11328

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 6 commits into from
Jun 14, 2018
Merged

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Jun 14, 2018

A bunch of small and somewhat orthogonal cleanups, all helping a little with #10151, and maybe #6103

@@ -2914,15 +2914,13 @@ def _setdef():


def extend_all(module):
adict = {}
for a in __all__:
adict[a] = 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.

Possibly written before set() was available...

@eric-wieser eric-wieser force-pushed the misc-numeric-cleanup branch from f43dd98 to ebce412 Compare June 14, 2018 05:50
@eric-wieser eric-wieser force-pushed the misc-numeric-cleanup branch from ebce412 to e2d83eb Compare June 14, 2018 05:54
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Even just the added comments are worth it! Looks good modulo a small nitpick.

# If two C types have the same size, then the earliest one in this list is used
# as the sized name.
_int_ctypes = ['long', 'longlong', 'int', 'short', 'byte']
_uint_ctypes = list('u' + t for t in _int_ctypes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Total nitpick, but just ['u' + t for t in _int_ctypes] is faster (and clearer)

Copy link
Member Author

Choose a reason for hiding this comment

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

A list comprehension would leak t into the module scope, unlike the above which does not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, had not thought about that - OK, would seem all good to go then!

Copy link
Member

Choose a reason for hiding this comment

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

Although I think that is fixed in recent Python

@eric-wieser
Copy link
Member Author

Follow-up at #10151

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.

3 participants