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

Added tests for input/output filenames for ICON and EMAC on-the-fly CMORizer #1718

Merged
merged 1 commit into from
Sep 12, 2022

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Sep 8, 2022

Description

While coding for #1678 I realized that we currently don't have any tests for input file names/input directories/output files for the EMAC and ICON on-the-fly CMORizers. This PR adds those. In additon, I slightly adapted the output file name for EMAC to make it consistent with CMIP data.

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@schlunma schlunma added this to the v2.7.0 milestone Sep 8, 2022
@schlunma schlunma self-assigned this Sep 8, 2022
@schlunma schlunma marked this pull request as ready for review September 8, 2022 14:17
@codecov
Copy link

codecov bot commented Sep 8, 2022

Codecov Report

Merging #1718 (cb70031) into main (62b5c30) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1718   +/-   ##
=======================================
  Coverage   91.47%   91.47%           
=======================================
  Files         206      206           
  Lines       11204    11204           
=======================================
  Hits        10249    10249           
  Misses        955      955           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@schlunma schlunma mentioned this pull request Sep 12, 2022
9 tasks
@schlunma
Copy link
Contributor Author

@valeriupredoi would you have time to briefly look at this? Thanks so much!!

@valeriupredoi
Copy link
Contributor

@schlunma cheers, yeps - will do today, in a jiffy!

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

looking good, cheers Manu! One question related to the native file names (sorry, first time I sees them), and another question - do we have true negative tests? ie a group of files that should not be found - I can't remember if I implemented that initially, so if there are none for CMIP data that's fine, we can leave it like that

@schlunma
Copy link
Contributor Author

This also has negative tests:

available_files:
      - amip/Amon/amip___________200001_Amon.nc
      - amip/Amon/amip___________200002_Amon.nc
      - amip/Amon/amip___________200003_Amon.nc
      - amip/Amon/amip___________200001_Amon-p-mm.nc
      - amip/Amon/amip___________200002_Amon-p-mm.nc
      - amip/Amon/amip___________200003_Amon-p-mm.nc
      - amip/rad/amip___________200001_rad.nc
      - amip/rad/amip___________200002_rad.nc
      - amip/rad/amip___________200003_rad.nc
      - amip/rad/amip___________200001_rad-p-mm.nc
      - amip/rad/amip___________200002_rad-p-mm.nc
      - amip/rad/amip___________200003_rad-p-mm.nc
    dirs:
      - amip/Amon
    file_patterns:
      - amip*Amon.nc
    found_files:
      - amip/Amon/amip___________200002_Amon.nc
      - amip/Amon/amip___________200003_Amon.nc

As you can see, only a subset of the files available is supposed to be found.

@valeriupredoi
Copy link
Contributor

This also has negative tests:

available_files:
      - amip/Amon/amip___________200001_Amon.nc
      - amip/Amon/amip___________200002_Amon.nc
      - amip/Amon/amip___________200003_Amon.nc
      - amip/Amon/amip___________200001_Amon-p-mm.nc
      - amip/Amon/amip___________200002_Amon-p-mm.nc
      - amip/Amon/amip___________200003_Amon-p-mm.nc
      - amip/rad/amip___________200001_rad.nc
      - amip/rad/amip___________200002_rad.nc
      - amip/rad/amip___________200003_rad.nc
      - amip/rad/amip___________200001_rad-p-mm.nc
      - amip/rad/amip___________200002_rad-p-mm.nc
      - amip/rad/amip___________200003_rad-p-mm.nc
    dirs:
      - amip/Amon
    file_patterns:
      - amip*Amon.nc
    found_files:
      - amip/Amon/amip___________200002_Amon.nc
      - amip/Amon/amip___________200003_Amon.nc

As you can see, only a subset of the files available is supposed to be found.

gotcha! Too Monday morning for me to catch that - good stuff! All fine by me then, cheers for the good work bud! 🍺

@schlunma
Copy link
Contributor Author

Thanks for the quick review 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants