Skip to content
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

fix 212 #234

Merged
merged 1 commit into from
Nov 3, 2015
Merged

fix 212 #234

merged 1 commit into from
Nov 3, 2015

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Nov 3, 2015

@BibMartin this is far from ideal because I am encoding only when there is no self.json_data. But I do not want to hold v0.1.6 and I want all our efforts to be on v0.2.0.

and float legend labels.
freescale: if True use free format for the scale, where min and max
values are taken from the data. It also allow to plot allow to plot
values < 0 and float legend labels.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there's a typo here, isn't it ?
you may keep the indent, and avoid the duplication of allow to plot

Copy link
Member Author

Choose a reason for hiding this comment

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

Urgh! My pep8 stupid mindset attacks again.

@BibMartin
Copy link
Contributor

Except the line 805-807 comment changes, I'm okay for the rest.
Merge soon

@BibMartin
Copy link
Contributor

Perfect. Squash ?

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 3, 2015

Done.

BibMartin added a commit that referenced this pull request Nov 3, 2015
@BibMartin BibMartin merged commit a0fc55d into python-visualization:v0.1.6 Nov 3, 2015
@BibMartin
Copy link
Contributor

Merged. 🔫

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 3, 2015

v0.1.6 is almost out of the windows 😌

@ocefpaf ocefpaf deleted the port_fix_212 branch November 3, 2015 15:27
@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 4, 2015

I am rethinking this one and I plan to revert it.

This is a bad solution and we should just tell to people to start migrating to v0.2.0.

The problems are:

  • It is inconsistent. I encode the HTML only when it does not have json_data. I should've used self.map_type = ='geojson' instead.
  • It breaks PNG, Fancy HTML (mpld3), and Vega popups.

I guess the downsides are not worth fixing #212.

@ocefpaf ocefpaf added the bug An issue describing unexpected or malicious behaviour label Feb 12, 2016
@ocefpaf ocefpaf modified the milestones: v0.2.0, v0.1.6 Feb 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue describing unexpected or malicious behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants