Skip to content

Conversation

@HGWright
Copy link
Contributor

@HGWright HGWright commented Jul 26, 2024

🚀 Pull Request

Description


Closes #4864

This adds the monthly and yearly options to guess_bounds.


Add any of the below labels to trigger actions on this PR:

  • benchmark_this Request that this pull request be benchmarked to check if it introduces performance shifts

@HGWright HGWright changed the title add monthly and some tests Adding monthly and yearly arguments to guess_bounds Jul 26, 2024
@codecov
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.77%. Comparing base (4585059) to head (372cf29).
Report is 57 commits behind head on main.

Files with missing lines Patch % Lines
lib/iris/coords.py 96.42% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6090      +/-   ##
==========================================
+ Coverage   89.75%   89.77%   +0.01%     
==========================================
  Files          88       88              
  Lines       22976    23011      +35     
  Branches     5022     5032      +10     
==========================================
+ Hits        20621    20657      +36     
+ Misses       1624     1623       -1     
  Partials      731      731              

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

@HGWright HGWright requested a review from pp-mo July 26, 2024 11:26
Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

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

Monthly looks good.
I checked the coverage + it is complete ✔️

Do you want to try for yearly too ?!?

@HGWright HGWright marked this pull request as ready for review July 26, 2024 15:06
Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

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

This is cool, one slight improvement suggested.

But .. I think we need to error if both monthly + yearly are True.
And maybe a mention of that in the docstring too.

Copy link
Member

@pp-mo pp-mo left a comment

Choose a reason for hiding this comment

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

Nice, I think this nails it all.
Good to have fixed on pytest now, I think -- a mixture of pytest + unittest is not so good !

@pp-mo pp-mo enabled auto-merge (squash) July 26, 2024 15:56
@pp-mo pp-mo merged commit 83905e9 into SciTools:main Jul 26, 2024
stephenworsley added a commit to stephenworsley/iris that referenced this pull request Jul 29, 2024
* main:
  Adding monthly and yearly arguments to `guess_bounds` (SciTools#6090)
tkknight added a commit to tkknight/iris that referenced this pull request Jul 30, 2024
* upstream/main:
  Load performance improvement (ignoring UGRID) (SciTools#6088)
  Adding monthly and yearly arguments to `guess_bounds` (SciTools#6090)
  Mesh nonexperimental extra (SciTools#6077)
  Mesh nonexperimental (SciTools#6061)
  Bump scitools/workflows from 2024.07.4 to 2024.07.5 (SciTools#6076)
  Updated environment lockfiles (SciTools#6068)
  Enable UGRID loading always; deprecate PARSE_UGRID_ON_LOAD. (SciTools#6054)
  Bump scitools/workflows from 2024.07.3 to 2024.07.4 (SciTools#6071)
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.

Should guess_bounds "do what I mean" for Gregorian monthly data?

2 participants