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

Remove setting backend to agg automatically #100

Merged
merged 3 commits into from
Apr 7, 2022

Conversation

freemansw1
Copy link
Member

Resolves #95.

@w-k-jones I think there's a chance that this could potentially break some code. For example, if a user tries to run tobac.plotting.plot_tracks_mask_field_loop in a script without setting their own backend, it could break. We could resolve it by putting the matplotlib imports into each function and setting mpl.use('agg') there, but I still think we're better off having the users set their own backend as agg isn't appropriate for all cases.

@freemansw1 freemansw1 added bug Code that is failing or producing the wrong result enhancement Addition of new features, or improved functionality of existing features labels Mar 28, 2022
@freemansw1 freemansw1 added this to the Version 1.3 milestone Mar 28, 2022
@freemansw1 freemansw1 requested a review from w-k-jones March 28, 2022 00:26
@w-k-jones
Copy link
Member

I agree, it's better to remove it to prevent unexpected knock-on effects for users. I think for now we can simply warn users that they may need to set a backend themselves (perhaps throw a warning when tobac.plotting is imported?) and deal with any problems if they appear. I think it would be best to make the plotting tools an optional import in future and remove matplotlib as a dependency (unless using plotting)

@freemansw1 freemansw1 requested a review from w-k-jones April 7, 2022 14:07
@freemansw1
Copy link
Member Author

Okay, I added warnings if agg isn't set as the backend where figures are rendered. I've also moved the import matplotlib.pyplot as plt import to each individual function that uses it to give us the opportunity to set the backend there if we feel that it is more appropriate.

As I've said in a few different venues now, I do wonder what the future of the plotting module (and to a lesser extent the analysis module) is. I think there are advantages to having easy tools available for new users, but at the moment they both rely extensively on iris tools and both need substantial refactoring to improve ease of use and documentation. I'm going to ponder this some and raise an issue about it. If we want to start depreciating functions, v1.4/1.5 is a good opportunity to do so.

@w-k-jones
Copy link
Member

Okay, I added warnings if agg isn't set as the backend where figures are rendered. I've also moved the import matplotlib.pyplot as plt import to each individual function that uses it to give us the opportunity to set the backend there if we feel that it is more appropriate.

As I've said in a few different venues now, I do wonder what the future of the plotting module (and to a lesser extent the analysis module) is. I think there are advantages to having easy tools available for new users, but at the moment they both rely extensively on iris tools and both need substantial refactoring to improve ease of use and documentation. I'm going to ponder this some and raise an issue about it. If we want to start depreciating functions, v1.4/1.5 is a good opportunity to do so.

I agree, I think that at the very least we should avoid needing matplotlib as a mandatory dependency. Both the plotting and analysis modules are heavily reliant on iris, so will obviously need big changes when we shift to xarray. I think we should keep both as they are for now, but at some point add warnings to note that these tools may not be available in version 2. I think that some plotting and analysis tools should remain, but more as examples for usage of tobac and for demonstrating how e.g. figures in papers and the examples notebooks are produced than as a core part of tobac.

Anyway, happy for this PR to be merged

@freemansw1 freemansw1 merged commit df0e5ff into tobac-project:dev Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code that is failing or producing the wrong result enhancement Addition of new features, or improved functionality of existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants