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

MAINT: drop climate.MapPlots in favour of climate.CartopyPlots #203

Closed
5 tasks done
fkuehlein opened this issue Oct 12, 2023 · 2 comments · Fixed by #217
Closed
5 tasks done

MAINT: drop climate.MapPlots in favour of climate.CartopyPlots #203

fkuehlein opened this issue Oct 12, 2023 · 2 comments · Fixed by #217
Assignees
Labels
maintenance something should be improved or is outdated
Milestone

Comments

@fkuehlein
Copy link
Collaborator

fkuehlein commented Oct 12, 2023

Look into deprecation/dropping of climate.MapPlots, as discussed in #196:

  • double check whether climate.MapPlots's functionality is fully covered by climate.CartopyPlots (at least basic features are covered according to @zugnachpankow)
  • delete climate/map_plots.py
  • rename CartopyPlots to MapPlot?
  • update all occurrences and tests of MapPlots
  • update README.rst accordingly by removing PyNGL and Matplotlib Basemap from 'optional dependencies'
@fkuehlein fkuehlein added the maintenance something should be improved or is outdated label Oct 12, 2023
@fkuehlein fkuehlein added this to the New Release milestone Oct 12, 2023
@fkuehlein fkuehlein self-assigned this Oct 12, 2023
@fkuehlein
Copy link
Collaborator Author

fkuehlein commented Dec 20, 2023

Looking into this a bit I found that CartopyPlots does not feature a couple of things that MapPlots did. However, I find this legitimate for the sake of simplicity, only that I would then find it consequent to make it even simpler. Here's an idea:

MapPlots features plotting multiple datasets to a single pdf file or to multiple pdf files, or just displaying them in a window.
CartopyPlots simply plots every dataset to a separate png-file. I feel like this basic plotting feature is totally sufficient, but then why even bother storing datasets in self.map_data? (note that self.map_mult_data is unused there) Why not just shortcut add_dataset() and generate_plots() and have something like:

class MapPlot:
  def __init__(self, grid, title):
    # initialise cartopy plot based on given Grid object

  def plot(self, title, data, file_name, title_on=True, labels_on=True):
    # plot given dataset on the Grid, then show and/or save to specified file

This would also resolve the question of intermediately storing datasets in lists or, for large datasets, npy-files, which was another extra feature of MapPlots. Beyond that, it might be nice to add the option to display the plot that is saved (sth like if keyword: plt.show()), and specify the file type (keeping png as default). Could also have separate plot() and savefig() methods, as in matplotlib ..

@ntfrgl, @jdonges, what do you think? @zugnachpankow, would you be up to implement such a thing?? Then we could finally substitute the apparently stale MapPlots class!

@fkuehlein
Copy link
Collaborator Author

update: @zugnachpankow and I plan to meet up in January to look into this together!

@fkuehlein fkuehlein linked a pull request Jan 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance something should be improved or is outdated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant