-
Notifications
You must be signed in to change notification settings - Fork 25
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
gull_tracking: modernized project example #357
Conversation
Azaya89
commented
Feb 2, 2024
•
edited by maximlt
Loading
edited by maximlt
- Modernized example
- Added explanations for the code
- Discuss hosting the file on our S3 due to the lack of the content-length in the response from Zenodo preventing any progress bar to be displayed by anaconda-project?
Your changes were successfully integrated in the dev site, make sure to review |
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.
However, I left the
lnglat_to_meters
function as is since I don't know the equivalent function from hvplot.
The utility to convert the coordinates is the HoloViews hv.util.transform.lon_lat_to_easting_northing function. You've seen it in use already in the HoloViz tutorial (https://holoviz.org/tutorial/Composing_Plots.html). Can you please update the code to use it instead?
I've left some other comments, good start! :)
Your changes were successfully integrated in the dev site, make sure to review |
1 similar comment
Your changes were successfully integrated in the dev site, make sure to review |
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 and clean, thanks!
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
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 feel it would be good to describe more the dataset before displaying it. I'd suggest showing the output of df.head()
and len(df)
. The output of df.describe()
wouldn't be very interesting, I think, as we only read the lat/lon columns (the CSV has more columns). The text says there are 2.4 million points in the dataset but as someone looking at this example I'd rather see the output of a code cell show that :)
I was a bit stumped by the fact that the map wasn't centered on the dataset. Running the notebook, I found out that there are 4 points farther West and North.
(These data points are actually displayed on the map, but they're very small and have a dark color. We use dynspread=True
to deal with this kind of case, maybe its settings like threshold
and max_px
need to be adjusted to better display these isolated points. @jbednar)
I think the example would look better if there was a plot focused on where most of the points are. For example, after removing these points (e.g. df = df[(df['location-long'] > -14) & (df['location-lat'] < 53.3)]
), the map looks better I think:
I stored a copy of the file on our S3 and updated the |
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
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.
LGTM! Thanks for the latest changes, merging :)