Skip to content

Add float to FacetValue and improve preprocessor filename template error message#2931

Merged
bouweandela merged 4 commits intomainfrom
better-preprocessor-filename-template-error-messages
Dec 19, 2025
Merged

Add float to FacetValue and improve preprocessor filename template error message#2931
bouweandela merged 4 commits intomainfrom
better-preprocessor-filename-template-error-messages

Conversation

@bouweandela
Copy link
Member

@bouweandela bouweandela commented Dec 18, 2025

Description

Add float to esmvalcore.typing.FacetValue and improve the error message when an error occurs creating a preprocessor filename from the template.

This is needed because some of the OBS/OBS6 datasets use a float as a version number and I hadn't realized that facet values could be floats when switching esmvalcore.typing.FacetValue from number.Number to int in https://github.com/ESMValGroup/ESMValCore/pull/2765/files#diff-09172b6abd0ac8654c50bab112192d3675ce773a649573a39d8ae9759866c184 because of python/mypy#3186.

Fixes part of ESMValGroup/ESMValTool#4293


Before you get started

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:

@codecov
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.59%. Comparing base (bf60005) to head (9a5121c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
esmvalcore/typing.py 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (92.85%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2931   +/-   ##
=======================================
  Coverage   95.59%   95.59%           
=======================================
  Files         266      266           
  Lines       15559    15569   +10     
=======================================
+ Hits        14873    14883   +10     
  Misses        686      686           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bouweandela bouweandela added the enhancement New feature or request label Dec 19, 2025
@bouweandela bouweandela marked this pull request as ready for review December 19, 2025 08:59
from iris.cube import Cube

FacetValue = str | Sequence[str] | int
FacetValue = str | Sequence[str] | int | float
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how to test this code

Copy link
Contributor

Choose a reason for hiding this comment

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

you can't - it's literally a typing declaration, they should be excluded from testing

f"for {dataset}, facet values have invalid type: "
+ ", ".join(
f"'{f}: {dataset.facets[f]}' has type {type(dataset.facets[f]).__name__}"
for f in sorted(wrong_type_facets)
Copy link
Contributor

Choose a reason for hiding this comment

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

may not want to sort so that the user knows what order those wrong facets came in

Copy link
Member Author

Choose a reason for hiding this comment

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

The way the facets arrive in a particular order isn't very transparent to users, so I'm not convinced we should put effort in preserving it. I expect it's something like the order from the recipe, depending on the project some facets may be added from the CMOR table and CV and then extra facets if they're defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah fair dos 🍺

@bouweandela
Copy link
Member Author

Thanks for reviewing!

@bouweandela bouweandela merged commit 2e39338 into main Dec 19, 2025
3 of 4 checks passed
@bouweandela bouweandela deleted the better-preprocessor-filename-template-error-messages branch December 19, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants