Skip to content

Conversation

@stephenworsley
Copy link
Contributor

@stephenworsley stephenworsley commented Dec 20, 2019

Align arithmetic with previous behaviour wrt CellMeasure

In the absence of changes to arithmetic due with #3478, cube arithmetic is given the "safe" behaviour of always discarding cell measures and ancillary variables. This aligns with the previous behaviour of arithmetic discarding cell measures (which was previously due to copy discarding cell measures). This solves one of the remaining issues in #3483 and allows loading of ancillary variables to be merged in safely if the canges made to cube arithmetic don't make it into 3.0.

@stephenworsley stephenworsley added this to the v3.0.0 milestone Dec 20, 2019
@pp-mo
Copy link
Member

pp-mo commented Dec 20, 2019

👍 LGTM
But not merging this yet.

As I understand it, we are holding this solution in reserve in case we want to "make arithmetic safe" for a possible future Iris 3.0 release not including #3478.
If we do complete #3478, and include that in our Iris 3.0, then this is probably no longer wanted.

@pp-mo pp-mo self-assigned this Dec 20, 2019
@stephenworsley stephenworsley changed the title Align arithmetic with previous behaviour wrt CellMeasure PI-3483 Align arithmetic with previous behaviour wrt CellMeasure Dec 20, 2019
@stephenworsley stephenworsley changed the title PI-3483 Align arithmetic with previous behaviour wrt CellMeasure PI-3483 (if not merging 3478) Safe arithmetic Dec 20, 2019
@bjlittle bjlittle self-requested a review December 20, 2019 20:03
Copy link
Member

@bjlittle bjlittle left a comment

Choose a reason for hiding this comment

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

@stephenworsley @pp-mo @abooton Tagging iris 3.0.0 without #3478 would be a major mistake.
I'm completely 👎 on this PR.

@bjlittle bjlittle closed this Dec 22, 2019
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.

4 participants