Skip to content

Remove use of six and __future__ imports#554

Merged
aaronayres35 merged 4 commits into
masterfrom
remove-six
Jan 19, 2021
Merged

Remove use of six and __future__ imports#554
aaronayres35 merged 4 commits into
masterfrom
remove-six

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 commented Jan 6, 2021

Part of #483

This PR removes all use of six and also removes all __future__ imports (only imports were for with_statement, print_function, and absolute_import).

It also drive by removes a few lines of commented out code

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

I want to take a second look at the changes from sm.zip to zip. Some of them feel off and I don't trust the test suite to catch all potential issues.

Comment thread chaco/axis_view.py
label="Main"),
Group(
Item("tick_color", label="Color", style="custom"),
#editor=EnableRGBAColorEditor()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'm happy when dead code is removed - but it'd be nice to mention this in the PR description.

@aaronayres35
Copy link
Copy Markdown
Contributor Author

I want to take a second look at the changes from sm.zip to zip. Some of them feel off and I don't trust the test suite to catch all potential issues.

I was following here: https://six.readthedocs.io/
Specifically in the supported renames section

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread chaco/legend.py
return [0, 0]

plot_names, visible_plots = list(sm.map(list, sm.zip(*sorted(self.plots.items()))))
plot_names, visible_plots = list(map(list, zip(*sorted(self.plots.items()))))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not a big fan of such code - list(map(list, zip(*sorted(self.plots.items())))) - because i find it hard to understand. Anyway, that's not an actionable item in this PR given the lack of context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, any code with ")))))" smells a bit, but that is a problem for another day/PR

@aaronayres35 aaronayres35 merged commit 2297206 into master Jan 19, 2021
@rahulporuri rahulporuri deleted the remove-six branch March 23, 2021 03:11
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.

2 participants