-
Notifications
You must be signed in to change notification settings - Fork 285
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
date_microseconds
FUTURE flag
#6260
date_microseconds
FUTURE flag
#6260
Conversation
for more information, see https://pre-commit.ci
⏱️ Performance Benchmark Report: 62e8bafPerformance shifts
Full benchmark results
Generated by GHA run |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6260 +/- ##
==========================================
- Coverage 89.83% 89.81% -0.02%
==========================================
Files 88 88
Lines 23315 23347 +32
Branches 4338 4344 +6
==========================================
+ Hits 20945 20970 +25
- Misses 1644 1649 +5
- Partials 726 728 +2 ☔ View full report in Codecov by Sentry. |
Linkcheck should be fixed by #6261 |
The lack of coverage is only because we're testing against cf-units version 3.2 in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in theory, though I think there's a way to achieve this without changing the behaviour of cf_units for code outside of iris by limiting this to a change in the Unit
class which iris objects have as their units
attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one. This looks a lot more robust now.
…ask_slicing_bug * origin/pin_dask_slicing_bug: `date_microseconds` FUTURE flag (SciTools#6260) [pre-commit.ci] pre-commit autoupdate (SciTools#6259) Remove freepik.com link due to linkcheck breakage (SciTools#6261)
Thanks @stephenworsley! |
* PoC monkeypatch precision. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add FUTURE flag. * FutureWarning. * Corrected behaviour and added tests. * Corrected behaviour and added tests. * What's New entry. * Make sensitive to cf-units version. * Further test improvements. * Clearer FutureWarning text. * Use a cf-units subclass instead. * Rename _IrisUnit to Unit. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
🚀 Pull Request
Description
#6256 proves that this protects our tests, and our users, against the new precision introduced in cf-units version 3.3. Unfortunately that PR is blocked by other dependency issues that will be addressed elsewhere, so I want to introduce these code changes onto
main
independently, which I have agreed with @stephenworsley.I will run the benchmarks to check whether my change was also responsible for the performance shifts in #6256.
Consult Iris pull request check list
Add any of the below labels to trigger actions on this PR: