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

Initial CMOR pytest examples #172

Merged
merged 29 commits into from
Sep 24, 2024
Merged

Initial CMOR pytest examples #172

merged 29 commits into from
Sep 24, 2024

Conversation

Ciheim
Copy link
Collaborator

@Ciheim Ciheim commented Aug 27, 2024

Describe your changes

Issue ticket number and link (if applicable)

Checklist before requesting a review

  • [ X] I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • [X ] I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • [X ] I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback

@Ciheim
Copy link
Collaborator Author

Ciheim commented Aug 27, 2024

#164

@Ciheim Ciheim changed the title #164 Early pytest (rough) (yes, I know my paths are there) Early pytest (rough) (yes, I know my paths are there) Aug 27, 2024
@ilaflott ilaflott marked this pull request as draft August 29, 2024 20:21
This was linked to issues Sep 10, 2024
@ilaflott ilaflott changed the title Early pytest (rough) (yes, I know my paths are there) Initial CMOR pytest examples Sep 12, 2024
ilaflott and others added 20 commits September 12, 2024 12:10
change string formation from algebraic approach to f-strings
break up some dense function calls
remove globals
adjust input arguments to permit global removal
lots of snake_case adjustments
add mysterious unused ierr var TODO item.
add in helpful printouts and whitespace for readability.
change most print statements w/ variable objects, exceptions, and path-formations to be driven by f-strings
add in a second-class entry point for direct module import of cmor run tool for programmatic usage and testing, such that the click entry point functions the way it is.
add in test for testing imported-module functionality of fre_cmor_run instead of just click-based cli-driven usage
remove unneeded click directives for using cmor_mixer.py directly as an executable script.
change order of defined functions such that it reflects the hierarchy of the scripts design
change function name "var2process" to something more understandable "gfdl_to_pcmdi_var"
realizing part of the dir structure is a YYYYMMDD datetime string
no return --> what to assert ...
nccmp weird via tests but maybe i dont have the dir right so im gonna try a fresh clone to tidy that up
_cmor_run_subtool argument order switch
identified section in cmor_mixer with my favorite gfdl'ism - (seemingly) senseless filemovent logic!

changed some opaque variable names in cmor_mixer
var_i --> gfdl_var
_tbl_ --> _table_
fl --> file
nm --> name
etc.
…assing nicely now

these tests wont pass on the pipeline unforutnately... dependence on cmip6-data-tables
... dependence on a netcdf file that has megabytes assoc with it
... dont want to blow up repo size... can we do an ncgen + cdl for the metadata?
…note down of where it comes frmo

add in a gitignore line ignoring other contents of that clone repository directory
tempted to just add 15MB of test file to the repo for FOREVER just because i wanna see a green circle
…e some pylint output to look at even if the tests fail
…r run subtool test. now does a git clone for the relevant cmip6 cmor table
…honGit as pip-package dependency... would be better to do this as a testing dependency specifically but nevermind that for now...
gitpython dependency in meta.yaml now encoded via the build script... not my favorite but it should work
removed gitpython pip mod dependency in meta.yaml... it does not work that way.
added in conda-forge::git as dependency for conda package / environment metadata
make it a quick command before claling pytest under commands: section/key
@ilaflott ilaflott marked this pull request as ready for review September 24, 2024 18:52
@ilaflott
Copy link
Member

tests/build passing etc.. still some clean up to do here and there but overall ready to review @Ciheim

@ilaflott ilaflott self-requested a review September 24, 2024 18:56
Copy link
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

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

did a little self-review to help organize my clean up efforts

.gitignore Show resolved Hide resolved
fre/cmor/README Outdated Show resolved Hide resolved
fre/cmor/__init__.py Show resolved Hide resolved
fre/tests/test_fre_cmor_cli.py Show resolved Hide resolved
fre/tests/test_files/varlist Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

bodies i see here:

  1. L36, why is CMIP6 in the directory structure twice?
  2. L38-L39, why is fre tacked on to the end of {OUTDIR} and before /{CMOR_CREATES_DIR} ?

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, the full output directory structure has a pattern reflected by a field/variable at the bottom of fre/tests/test_files/CMOR_input_example.json... this should be checked explicitly!

Copy link
Member

@ilaflott ilaflott Sep 24, 2024

Choose a reason for hiding this comment

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

@Ciheim @ceblanton this is not fun but i know we want to get this test in and running so other devs can start working with the tool.

@ilaflott ilaflott self-requested a review September 24, 2024 20:42
Copy link
Member

@ilaflott ilaflott left a comment

Choose a reason for hiding this comment

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

i'm happy.

@ilaflott ilaflott merged commit 20683a9 into main Sep 24, 2024
2 checks passed
@ilaflott ilaflott deleted the 164.InitalCMORPytest branch September 24, 2024 20:43
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.

fre cmor run: desired tweaks fre cmor run pytest case
2 participants