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

Deprecate Basemap dependency #180

Merged
merged 16 commits into from
Nov 21, 2020
Merged

Deprecate Basemap dependency #180

merged 16 commits into from
Nov 21, 2020

Conversation

dnerini
Copy link
Member

@dnerini dnerini commented Nov 8, 2020

This PR removes the support for the Basemap module. As a consequence, all drawing of geographical features will be based on Cartopy only.

In detail:

  • Add drawlonlatlabels option (closes Axis tick labels with lon-lat lines #14)
  • Include additional geographical features (reefs, minor islands) when scale in ["10m", "50M"].
  • Passing geodata without Cartopy installed silently fails, producing a georeferenced plot without any basemap (which is equivalent to pass plot_map=None).
  • Add support for Geostationary and AzimuthalEquidistant projections.

See below for plots using pysteps example data:

test_plt_cartopy_2
test_plt_cartopy_3
test_plt_cartopy_4
test_plt_cartopy_5
test_plt_cartopy_6
test_plt_cartopy_7
test_plt_cartopy_8

@dnerini dnerini added this to the release v1.4 milestone Nov 8, 2020
@dnerini dnerini linked an issue Nov 8, 2020 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Nov 8, 2020

Codecov Report

Merging #180 (6f1a15e) into master (b88b1b2) will increase coverage by 0.40%.
The diff coverage is 65.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   74.40%   74.80%   +0.40%     
==========================================
  Files         110      110              
  Lines        8802     8736      -66     
==========================================
- Hits         6549     6535      -14     
+ Misses       2253     2201      -52     
Impacted Files Coverage Δ
pysteps/visualization/animations.py 5.68% <10.00%> (+1.08%) ⬆️
pysteps/visualization/motionfields.py 71.66% <46.66%> (-0.99%) ⬇️
pysteps/tests/test_plt_precipfields.py 88.88% <50.00%> (-0.40%) ⬇️
pysteps/visualization/utils.py 51.61% <63.63%> (+7.64%) ⬆️
pysteps/visualization/precipfields.py 66.29% <66.66%> (-2.52%) ⬇️
pysteps/tests/test_plt_cartopy.py 84.21% <83.33%> (ø)
pysteps/visualization/basemaps.py 81.81% <83.33%> (+32.39%) ⬆️
pysteps/nowcasts/sseps.py 87.52% <100.00%> (ø)
pysteps/tests/test_plt_motionfields.py 88.23% <100.00%> (+0.96%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b88b1b2...6f1a15e. Read the comment docs.

@dnerini dnerini marked this pull request as ready for review November 8, 2020 14:23
Copy link
Contributor

@RubenImhoff RubenImhoff 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, @dnerini! Looks ready to go!

@pulkkins
Copy link
Member

Nice work! I'm wondering should we still keep the plot_map option in the interface. In addition to cartopy, there are quite many map plotting tools that could be supported in future releases of pysteps.

@dnerini
Copy link
Member Author

dnerini commented Nov 11, 2020

I'm wondering should we still keep the plot_map option in the interface. In addition to cartopy, there are quite many map plotting tools that could be supported in future releases of pysteps.

It's a good point, it would also improve back-compatibility and provide a clear way to not plot a basemap (plot_map=None). I'll make the change asap.

edit: done in e5fed04

@dnerini dnerini merged commit e5e4a2e into master Nov 21, 2020
@dnerini dnerini deleted the deprecate-basemap branch November 23, 2020 12:05
@dnerini dnerini linked an issue Dec 10, 2020 that may be closed by this pull request
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.

Deprecate basemap Axis tick labels with lon-lat lines
3 participants