Skip to content

Fix #928 and implements from flake8 fixes that stickler missed #1017

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Nov 14, 2018
Merged

Fix #928 and implements from flake8 fixes that stickler missed #1017

merged 10 commits into from
Nov 14, 2018

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Nov 9, 2018

Closes #928

I also ported some functions from branca.utilitiesthat should live infolium.utilities`.

ocefpaf
ocefpaf commented Nov 9, 2018
View reviewed changes
@ocefpaf ocefpaf added the waiting for review PR is waiting to be reviewed label Nov 12, 2018
@ocefpaf ocefpaf requested a review from Conengmo November 12, 2018 20:49
Conengmo
Conengmo reviewed Nov 13, 2018
View reviewed changes
Copy link
Member

@Conengmo Conengmo left a comment

Choose a reason for hiding this comment

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

Nice work @ocefpaf, I like the clean up and the consistency.

I placed a few remarks/questions, hope you have time to comment on them.

@Conengmo Conengmo added in discussion This PR or issue is being discussed and removed waiting for review PR is waiting to be reviewed labels Nov 13, 2018
Conengmo
Conengmo approved these changes Nov 13, 2018
View reviewed changes
Copy link
Member

@Conengmo Conengmo left a comment

Choose a reason for hiding this comment

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

Alright seems good to me. I'll wait a few hours in case anyone has comments still and then merge it.

@Conengmo Conengmo added ready PR is ready for merging and removed in discussion This PR or issue is being discussed labels Nov 13, 2018
@Conengmo Conengmo merged commit 5706269 into python-visualization:master Nov 14, 2018
@Conengmo Conengmo removed the ready PR is ready for merging label Nov 14, 2018
@ocefpaf ocefpaf deleted the fix_928 branch November 15, 2018 12:11
@Conengmo Conengmo mentioned this pull request Nov 25, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Conengmo Conengmo Conengmo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ocefpaf @Conengmo