Skip to content

Cleanup unused variables #16097

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
Jan 6, 2020
Merged

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jan 4, 2020

PR Summary

Cleanup:

  • unused local variables
  • unused parameters of internal functions

@timhoffm timhoffm added this to the v3.3.0 milestone Jan 4, 2020
vspaces = [[] for i in range((rows + 1) * cols)]
hspaces = [[] for i in range(rows * (cols + 1))]
vspaces = [[] for _ in range((rows + 1) * cols)]
hspaces = [[] for _ in range(rows * (cols + 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.

Initially, I tried to shorten these further to

vspaces = [[]] * ((rows + 1) * cols)
hspaces = [[]] * (rows * (cols + 1))

But for some reason, this caused test failures.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, it is not the same thing:

>python -c "a = [[]] * 2; a[0].append(123); print(a)"
[[123], [123]]

P.S: I think those can be turned into numpy arrays of python lists.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, of course it's not the same. 🙄

I leave numpy-fication for later. I suspect one could use numpy also further down to speed things up a little, but it needs a closer look.

This includes local variables and parameters of internal functions.
@QuLogic QuLogic merged commit 42d4f03 into matplotlib:master Jan 6, 2020
@timhoffm timhoffm deleted the cleanup-unused branch January 6, 2020 10:32
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