Skip to content

Time dynamic geo json #741

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 35 commits into from
Oct 24, 2017
Merged

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Oct 6, 2017

Same as #736 with some minor fixes.

Ping @halfdanrump

halfdanrump and others added 30 commits June 23, 2017 15:20
raise ValueError('Each item in styledict must be a dictionary, got {!r}'.format(val))


# Make set of timestamps.

Choose a reason for hiding this comment

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

E303 too many blank lines (2)

assert isinstance(figure, Figure), ('You cannot render this Element '
'if it is not in a Figure.')
figure.header.add_child(
JavascriptLink('https://d3js.org/d3.v4.min.js'),

Choose a reason for hiding this comment

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

E122 continuation line missing indentation or outdented

'if it is not in a Figure.')
figure.header.add_child(
JavascriptLink('https://d3js.org/d3.v4.min.js'),
name='d3v4'

Choose a reason for hiding this comment

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

E122 continuation line missing indentation or outdented

super(TimeDynamicGeoJson, self).__init__(data, **kwargs)
assert isinstance(styledict, dict), 'styledict must be a dictionary'
class TimeSliderChoropleth(GeoJson):
def __init__(self, data, styledict, name=None, overlay=True, control=True, **kwargs):
Copy link
Member Author

Choose a reason for hiding this comment

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

Still need some docs.

for val in styledict.values():
assert isinstance(val, dict), 'each item in styledict must be a dictionary'
if not isinstance(val, dict):
raise ValueError('Each item in styledict must be a dictionary, got {!r}'.format(val))
Copy link
Member Author

Choose a reason for hiding this comment

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

raise instead of assert.

self.timestamps.update(set(feature.keys()))
self.timestamps = sorted(list(self.timestamps))
self.timestamps = json.dumps(timestamps)
self.styledict = json.dumps(styledict, sort_keys=True, indent=2)
Copy link
Member Author

Choose a reason for hiding this comment

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

By dumping json strings we avoid the u'' in the code and fix the py27 failures.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice

"'1472569200', '1475161200', '1477839600'];")
#assert expected_timestamps in out
# We verify that data has been inserted correctly
expected_timestamps = """var timestamps = ["1454198400", "1456704000", "1459382400"];""" # noqa
Copy link
Member Author

Choose a reason for hiding this comment

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

I reduced this to make it easier to compare.
Not to self: these values are not the same on Travis-CI and locally. I need to double check this!

@ocefpaf ocefpaf mentioned this pull request Oct 6, 2017
@halfdanrump
Copy link
Contributor

@ocefpaf I see a lot of people are submitting PRs, so you're probably pretty swamped :). Is there anything I can do to wrap up this PR?

Cheers,
Halfdan

@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 21, 2017 via email

@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 24, 2017

@halfdanrump I am merging this but we could use a re-write of the docs later.

Thanks for the PR!

@ocefpaf ocefpaf merged commit b3f31bb into python-visualization:master Oct 24, 2017
@ocefpaf ocefpaf deleted the TimeDynamicGeoJson branch October 24, 2017 13:57
sanga pushed a commit to sanga/folium that referenced this pull request Oct 24, 2017
@halfdanrump
Copy link
Contributor

@ocefpaf Got it, I'll add docs after the PR is complete and then issue a new PR :). Enjoy your trip!

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.

3 participants