Derived variables structure update#326
Open
justin-richling wants to merge 17 commits intoNCAR:mainfrom
Open
Conversation
Pull these checks and calculations out of `adf-diag.py` to clean that file up.
Now call the `adf_derive.py` script for derived variables
nusbaume
requested changes
Sep 24, 2024
Collaborator
nusbaume
left a comment
There was a problem hiding this comment.
Nice cleanup! Just have a few suggestions.
Comment on lines
+103
to
+104
| else: | ||
| self.debug_log(constit_errmsg) |
Collaborator
There was a problem hiding this comment.
Instead of checking whether constit_list is a list type below, would it possibly be safer to just add a logical that skips writing the constit_errmsg if you have already written it here?
| # End if | ||
|
|
||
| # Log if this variable can be derived but is missing list of constituents | ||
| if isinstance(constit_list, list) and not constit_list: |
Collaborator
There was a problem hiding this comment.
If you use the logical method suggested above, then you can probably get rid of the isinstance check here.
Pull these checks and calculations out of `adf-diag.py` to clean that file up.
Now call the `adf_derive.py` script for derived variables
b0e0067 to
fafd70d
Compare
…ing/ADF into derived-vars-update
This is now being generated in `adf_derive.py`
This file contains hidden or 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
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.
Move the calculation of derived variables out of
adf_diag.pyinto an external script. This is a placeholder until a more robust variable derivations scheme is implemented.This will help declutter
adf_diag.pyin the hopes of keeping non-essential methods separate.adf_derive.pyhas two main methods:check_deriveandderive_variablecheck_derive- called in variable loop; will check on each available variableFor incoming variable, look for list of constituents if available as a list in variable defaults yaml file
If the variable does not have the argument
derivable_fromorderivable_from_cam_chem,then it will be assumed not to be a derivable variable, just missing from history file
If the variable does have the argument
derivable_fromorderivable_from_cam_chem,first check cam-chem, then regular cam and add all constituents to
diag_var_listfor time series generation.NOTE: this will only modify
diag_var_listfor use inadf_diag.py, it will not modify the yaml variable list used in other scripts.derive_variable- called after variable loop and time series generation; loop over dictionary containing derived variables as keys and list of constituents as values.This will then be called in a loop of accepted derived variables and will be generated from constituent time series files.