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.

Sign up for GitHub

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

Jump to bottom

Update in driver.py to make ts_avg_window Optional, works for both ts_avg_window specified in yaml or not #183

Merged
merged 20 commits into from
Jul 17, 2023
Merged

Update in driver.py to make ts_avg_window Optional, works for both ts_avg_window specified in yaml or not #183

merged 20 commits into from
Jul 17, 2023

Conversation

quaz115
Copy link
Collaborator

@quaz115 quaz115 commented Jul 11, 2023 •
edited
Loading

Earlier ts_avg_window was needed in 'timeseries' plot type (e.g., 'H' or 'D' for hourly or daily averages), and didn't work with None specified to ts_avg_window.

This would be more useful for aircraft evaluations (looking at timeseries with altitude profile data, without the need of time window averaging)

@quaz115
Update in driver.py to make ts_avg_window Optional, works for cases e…
723ef43 
…ither ts_avg_window is specified in yaml or not
@quaz115 quaz115 requested a review from rschwant July 11, 2023 22:39
@quaz115 quaz115 requested a review from zmoon July 11, 2023 23:24
zmoon
zmoon reviewed Jul 11, 2023
View reviewed changes
Copy link
Collaborator

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

Appreciate you taking the time to update all the comments.

melodies_monet/driver.py Outdated Show resolved Hide resolved
examples/yaml/control_cmaq-rrfs_surface-all-short_test_jupyter.yaml Outdated Show resolved Hide resolved
docs/appendix/yaml.rst Outdated Show resolved Hide resolved
@zmoon
Copy link
Collaborator

zmoon commented Jul 11, 2023

I'll see if I can fix the CI tomorrow.

@zmoon
Copy link
Collaborator

zmoon commented Jul 14, 2023

@quaz115 I was able to fix the CI in #184 so if you merge upstream develop branch to your develop branch it should work properly now.

@quaz115
Copy link
Collaborator Author

quaz115 commented Jul 14, 2023

@quaz115 I was able to fix the CI in #184 so if you merge upstream develop branch to your develop branch it should work properly now.

@zmoon Done, all 4 tests passed now

@quaz115
Copy link
Collaborator Author

quaz115 commented Jul 15, 2023

@quaz115 I was able to fix the CI in #184 so if you merge upstream develop branch to your develop branch it should work properly now.

@zmoon Done, all 4 tests passed now

@zmoon @rschwant is this ready to be merged to https://github.com/NOAA-CSL/MELODIES-MONET/tree/develop branch now

@zmoon
Copy link
Collaborator

zmoon commented Jul 15, 2023

@quaz115 I think we should give @rschwant a bit more chance to take a look since she is the lord of control file options and their documentation.

rschwant
rschwant approved these changes Jul 17, 2023
View reviewed changes
Copy link
Collaborator

@rschwant rschwant left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for updating the docs too!

@quaz115 quaz115 merged commit 6b037b6 into NOAA-CSL:develop Jul 17, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zmoon zmoon zmoon left review comments

@rschwant rschwant rschwant approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quaz115 @zmoon @rschwant