Draft: Modify extract_season to return only "full" seasons upon request
#2597
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2597 +/- ##
=======================================
Coverage 94.96% 94.97%
=======================================
Files 253 253
Lines 14701 14716 +15
=======================================
+ Hits 13961 13976 +15
Misses 740 740 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
many thanks @k-a-webb - it's always good practice to have a Draft PR when not ready for review, as you mentioned in the PR title (so I went ahead and made it a Draft for you 👍 ) |
…st months of the season being outside of first/least years
|
@valeriupredoi, could you please help me understand why the pre-commit.ci check is failing? |
| season: | ||
| Season to extract. Available: DJF, MAM, JJA, SON | ||
| and all sequentially correct combinations: e.g. JJAS | ||
| full: |
There was a problem hiding this comment.
I would add more documentation or maybe context. I know what the issue is, but I think a person without context might get confused.
| full_season_coord, data_dims=result.coord_dims("time") | ||
| ) | ||
|
|
||
| result = result.extract(iris.Constraint(full_season=1)) |
There was a problem hiding this comment.
Personal preference, rather than need, but I would add a logger message in the debug format which months have been removed. Also on a similar note, in case the values are existing in a timeseries, but have been masked out, as far as I understand they won't be removed. I would recommend to add this information somewhere.
| with assert_raises(iris.exceptions.CoordinateNotFoundError): | ||
| self.cube.coord("season_year") | ||
|
|
||
| def test_get_djf_full(self): |
There was a problem hiding this comment.
Again, personal preference, rather than requirement, but I would add a second test when you provide a cube, where the season is captured fully (let's say from Dec to Nov) with the keyword full=True and check that the returned cube is correct.
|
In order to maintain a backlog of relevant pull requests, we automatically label them as stale after 180 days of inactivity. If this pull request is still important to you, please comment below to remove the stale label. Otherwise, this pull request will be automatically closed in 60 days. If this pull request only suffers from a lack of reviewers, please tag the @ESMValGroup/technical-lead-development-team so they can help you find a suitable reviewer. |
|
This stale pull request has been automatically closed due to a lack of community activity. If you still care about this pull request, please re-open it. If you do not have sufficient permissions for this, tag the @ESMValGroup/technical-lead-development-team in a comment below including a short message that this pull request is still relevant to you. They will re-open it on your behalf. |
Description
Related issue: #2591
Include a parameter
full(I'm open to this being a different word) which triggersextract_seasonto only return "complete" seasons. For example, "DJF" and never just "JF". Invokesextract_timeto clip the time vector prior to season extractionCloses #issue_number
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: