Skip to content

Conversation

@nocollier
Copy link
Contributor

@nocollier nocollier commented Jan 22, 2025

Description

Realized that I had missed some things needed for full integration.

Checklist

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Changelog item added to changelog/

@nocollier
Copy link
Contributor Author

@lewisjared I discovered that I missed a good bit in the integration which meant tests ran but didn't include ilamb things. Now I have some errors, but these last ones I don't understand. Do you have any idea of what I have missed?

@lewisjared
Copy link
Contributor

Yup it turns out the tests are sensitive to new metrics and sample data so need to be tweaked each time either changes. I've got a fix for this in a pr I'm about to merge so hopefully that will resolve this #68

@lewisjared
Copy link
Contributor

The changes look good though so feel free to assign it to me so I can merge once I fix the flakey test


name = "Global Mean Timeseries"
slug = "global-mean-timeseries"
name = "ILAMB Global Mean Timeseries"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that the slug had to be globally unique. The intention was that it is unique for a provider. The constraint is that the combination of the provider slug + the metric slug was unique

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a bunch (~10) failed tests due to some errors which reported 'unique slug' problems that were fixed when I made the change. But I will check this again.

@lewisjared
Copy link
Contributor

Testing this locally after merging main looks to work

git checkout missing-ilamb
git merge origin/main
make virtual-environment
make test

@codecov
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
...etrics-ilamb/src/cmip_ref_metrics_ilamb/example.py 100.00% <100.00%> (ø)
packages/ref/src/cmip_ref/provider_registry.py 92.59% <100.00%> (+0.59%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nocollier nocollier merged commit b58432c into main Jan 23, 2025
13 checks passed
@nocollier nocollier deleted the missing-ilamb branch January 23, 2025 15:01
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.

3 participants