Skip to content
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

Bugfix for #3696 #5382

Merged
merged 7 commits into from Jul 26, 2023
Merged

Bugfix for #3696 #5382

merged 7 commits into from Jul 26, 2023

Conversation

acchamber
Copy link
Contributor

🚀 Pull Request

Description

Following #3696, which left the user confused about how concatenation interfaced with cube overlap, added a more explicit error message if you try to use concatenate_cube to concatenate two cubes with overlapping coordinates and added warning if you use regular concatenate so the user understands why they remain separate.

New test added for the error message and all tests pass.


Consult Iris pull request check list

Copy link
Contributor

@stephenworsley stephenworsley left a comment

Choose a reason for hiding this comment

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

Looking good, there's a few things here to change. I also want to have a think about what's happening in the different cases of points vs bounds.

lib/iris/_concatenate.py Outdated Show resolved Hide resolved
lib/iris/_concatenate.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01% 🎉

Comparison is base (5b42f47) 89.37% compared to head (ae91dad) 89.39%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5382      +/-   ##
==========================================
+ Coverage   89.37%   89.39%   +0.01%     
==========================================
  Files          89       89              
  Lines       22426    22433       +7     
  Branches     5379     5381       +2     
==========================================
+ Hits        20043    20053      +10     
+ Misses       1637     1635       -2     
+ Partials      746      745       -1     
Files Changed Coverage Δ
lib/iris/_concatenate.py 96.14% <100.00%> (+0.67%) ⬆️

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

Copy link
Contributor

@stephenworsley stephenworsley left a comment

Choose a reason for hiding this comment

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

Looks good! I think one last thing I forgot to mention was a what's new entry (to go somewhere here, probably in "features"). This can either go in this PR or as part of its own PR (which is sometimes worth doing if there are many simultaneous PRs going in which could cause each other conflicts). Otherwise I'm happy to pull this one in.

Copy link
Contributor

@stephenworsley stephenworsley left a comment

Choose a reason for hiding this comment

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

Good stuff, I think we can merge this in now.

@stephenworsley stephenworsley merged commit 66ecb4c into SciTools:main Jul 26, 2023
17 checks passed
@acchamber acchamber deleted the bugfix_for_#3696 branch July 27, 2023 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants