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

Tr/minor tempest regridder changes #140

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

imreddyTeja
Copy link
Contributor

Purpose

  • Make TempestRegridder support multiple vars (maybe)
  • Make outfile_root a kwarg for TempestRegridder
  • improve on read_available_dates
  • Add tesst for TempestRegridder and read_available_dates

To-do

Content


  • I have read and checked the items on the review checklist.

@@ -72,10 +72,10 @@ function Regridders.TempestRegridder(
input_file::AbstractString;
regrid_dir::AbstractString,
mono::Bool = true,
outfile_root::AbstractString = "cgll",
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no real need for this to be a kwarg, or outfile_root to be changed in general.

Files saved during Regridding currently have a name structure as

hd_outfile_root * "_" * varname * "_" * string(time) * ".hdf5"

but hd_outfile_root is automatically set to varname, so the path becomes

varname * "_" * varname * "_" * string(time) * ".hdf5"

, which seems odd to me. The changes/potential changes in this PR came up in this PR

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay! I guess we can directly removed the hd_outfile_root then

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