-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Time dynamic geo json #741
Conversation
raise ValueError('Each item in styledict must be a dictionary, got {!r}'.format(val)) | ||
|
||
|
||
# Make set of timestamps. |
There was a problem hiding this comment.
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'), |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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): |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 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, |
I'm traveling at the moment but I'll tackle this one first once I get back.
…On Oct 20, 2017 8:48 PM, "Halfdan Rump" ***@***.***> wrote:
@ocefpaf <https://github.com/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
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#741 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA6BL-VRkdaS4HGhzzayuPhs-wcJr4jIks5suTFUgaJpZM4Pv4jN>
.
|
@halfdanrump I am merging this but we could use a re-write of the docs later. Thanks for the PR! |
…eoJson Time dynamic geo json
@ocefpaf Got it, I'll add docs after the PR is complete and then issue a new PR :). Enjoy your trip! |
Same as #736 with some minor fixes.
Ping @halfdanrump