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

Flexible method for isobaric slice plotting #8

Merged
merged 12 commits into from
Jun 8, 2022

Conversation

robinbaeyens
Copy link
Collaborator

I made the first plotting method, i.e. isobaric slice plotting.
I hope it is flexible enough to build other -- more specific -- plotting methods on it. Let me know what you think @AaronDavidSchneider

ps: first pull request in my life, yay!

Copy link
Contributor

@AaronDavidSchneider AaronDavidSchneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the plotting routines. They give an easy to use first quick and dirty plotting feeling!

I think that (apart from the comments) they are not even very restrictive.

One fundamental design choice for the plotting routines would be the question of weither we want the gcmt object to become a member of the Plotting class. A benefit of that could be that you could just select a tag to plot.

What do you think?

robinbaeyens and others added 2 commits June 7, 2022 16:29
Co-authored-by: Aaron David Schneider <aaron.schneider@nbi.ku.dk>
@robinbaeyens
Copy link
Collaborator Author

One fundamental design choice for the plotting routines would be the question of weither we want the gcmt object to become a member of the Plotting class. A benefit of that could be that you could just select a tag to plot.

Do you mean whether to do something like gcmt.get_models('HD20').isobaric_slice(...) or like isobaric_slice('HD20', ...), or both, or neither? :)
In any case, the current approach is for me the most intuitive, but I'm open to have it in a different way as well.

@AaronDavidSchneider
Copy link
Contributor

AaronDavidSchneider commented Jun 7, 2022

Do you mean whether to do something like gcmt.get_models('HD20').isobaric_slice(...) or like isobaric_slice('HD20', ...), or both, or neither? :) In any case, the current approach is for me the most intuitive, but I'm open to have it in a different way as well.

How about having both? E.g.
gcmt.isobaric_slice(tag='HD2') and
isobaric_slice(ds)

and then drop the Plotting class completely (no common methods, variables needed) and instead just have the plotting functions that can also be wrapped in gcmt?

Copy link
Contributor

@AaronDavidSchneider AaronDavidSchneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! I have two very small comments

GCMtools/gcm_plotting.py Outdated Show resolved Hide resolved
GCMtools/gcm_plotting.py Outdated Show resolved Hide resolved
robinbaeyens and others added 2 commits June 8, 2022 14:56
Co-authored-by: Aaron David Schneider <aaron.schneider@nbi.ku.dk>
Co-authored-by: Aaron David Schneider <aaron.schneider@nbi.ku.dk>
@AaronDavidSchneider AaronDavidSchneider merged commit 55535e2 into main Jun 8, 2022
@AaronDavidSchneider AaronDavidSchneider deleted the Robins_plotting branch June 8, 2022 13:33
@AaronDavidSchneider
Copy link
Contributor

Thanks @robinbaeyens!

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.

2 participants