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

Document surface obs xarray/nc format #346

Merged
merged 5 commits into from
Mar 11, 2025
Merged

Conversation

zmoon
Copy link
Collaborator

@zmoon zmoon commented Mar 10, 2025

No description provided.

@zmoon zmoon requested review from rschwant and blychs March 10, 2025 20:16
@zmoon
Copy link
Collaborator Author

zmoon commented Mar 10, 2025

Question: should we also add something here about getting new obs data sources into the tool "officially"? (e.g. start by opening Issue or Discussion in MM repo)

time_local is only really required if regulatory metrics are to be calculated. For anything else, the time variable to be used in the plot can be set in the YAML file.
@blychs
Copy link
Collaborator

blychs commented Mar 10, 2025

This looks great. I would reserve the "officially" for the developer's guide, but if you prefer to add them here, I don't see a problem with that either.
Just suggested a small change (time_local is only really required for regulatory metrics, for anything else the time variable to be used can already be set in the YAML file as ts_select_time)

@zmoon
Copy link
Collaborator Author

zmoon commented Mar 10, 2025

@blychs using time_local is also an option for the time series plot, used in many of the examples (plots.<group>.data_proc.ts_select_time: 'time_local'), so we may want to mention that as well.

zmoon and others added 2 commits March 10, 2025 17:10
Include mention of timeseries
Copy link
Collaborator

@blychs blychs left a comment

Choose a reason for hiding this comment

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

adding mentions of timeseries

@blychs
Copy link
Collaborator

blychs commented Mar 10, 2025

I'm ready to approve this PR once the one open conversation is closed.

Copy link
Collaborator

@blychs blychs 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.

@blychs
Copy link
Collaborator

blychs commented Mar 11, 2025

I'm happy for this to get merged @rschwant

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.

I agree, looks good! @zmoon you can merge it when ready.

@zmoon zmoon merged commit 031d308 into NOAA-CSL:develop Mar 11, 2025
5 checks passed
@zmoon zmoon deleted the doc-obs-nc branch March 11, 2025 21:27
@zmoon
Copy link
Collaborator Author

zmoon commented Mar 11, 2025

Thanks @blychs for the help

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.

3 participants