search

rom-py / rompy

Relocatable Ocean Modelling in PYthon (rompy) combines templated cookie-cutter model configuration with various xarray extensions to assist in the setup and evaluation of coastal ocean model
https://rom-py.github.io/rompy/
BSD 3-Clause "New" or "Revised" License
3 stars 9 forks source link

plotting used old latname lonname instead of coords, also made cartopy a package requirement #31

Closed benjaminleighton closed 1 year ago

rafa-guedes commented 1 year ago

Looks good @benjaminleighton thanks for fixing those. Just regarding cartopy, is it necessary for it to be a required dependency? It is currently defined as an "extra" dependency (meaning if we install rompy as pip install "rompy[extra]" we will get cartopy installed. I think cartopy is always imported inside plotting functions at the moment to make sure rompy doesn't break if it is not installed. I feel it should be okay for it to be a required dep - it is a lot easier to install it nowadays than it used to be a couple of years ago, but just to make sure we are all happy.

benjaminleighton commented 1 year ago

Thanks very much @rafa-guedes, I didn't know or see that. I'm happy to go ahead as you advise either keeping it or removing it. Should you advise removing the requirement then I it would probably make sense to remove the test too? The test might be a bit excessive anyhow?

rafa-guedes commented 1 year ago

I'd be happy for it to stay - I just double checked it on a clean environment and cartopy does not add that much more after all the other rompy dependencies have been installed. I'll just remove it from the "extra" list and merge then!