Tr/minor tempest regridder changes #140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
Sbozzolo
reviewed
Dec 12, 2024
ext/TempestRegridderExt.jl
Outdated
| @@ -72,10 +72,10 @@ function Regridders.TempestRegridder( | |||
| input_file::AbstractString; | |||
| regrid_dir::AbstractString, | |||
| mono::Bool = true, | |||
| outfile_root::AbstractString = "cgll", | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Ah okay! I guess we can directly removed the hd_outfile_root then
There was a problem hiding this comment.
I agree it's okay to remove it. It would be good to have an organized naming of regridded files when/if we support regridding a file that contains multiple variables. But that can be accomplished using regrid_dir rather than outfile_root.
imreddyTeja
force-pushed
the
tr/minor-tempest-regridder-changes
branch
3 times, most recently
from
24a4192 to
16eaf38
Compare
juliasloan25
approved these changes
Dec 16, 2024
This was automatically set to `varname`, which resulted in save paths of the form varname_varname_time.hdf5. This path is never exposed to the user, so there is no interface change.
Add very simple TempestRegridder test Add test for read_available_dates
The functionality of the `yyyymmdd_to_datetime` function is already contained in the Dates package.
imreddyTeja
force-pushed
the
tr/minor-tempest-regridder-changes
branch
from
16eaf38 to
991b2df
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Make TempestRegridder support multiple vars (maybe)This is likely not needed. It should be its own PR if we ever decide it is needed, and the implementation/interface requires planning to reflect the design of InterpolatonsRegridderMake outfile_root a kwarg for TempestRegridderJust remove it.improve on read_available_datesyyyymmdd_to_datetimewith Dates built in parsingTo-do
Content