iris

iris copied to clipboard

Aiming to address #3616

Although this is technically in progress, it isn't necessarily going to go into Iris 3.2. So to avoid it cluttering the "in progress" column, I will move it to to do. We can then pick it up if we time.

If we do get to it then we'll need a rebase onto main before it can be reviewed I think

@pp-mo are you able to finish this one off before getting back into #5016? Sounds like this is pretty close.

Rebased. but N.B. put in draft temporarily

It is not good to go, since the auto-merged result is still using Coord._as_defn() And we now want to use common metadata instead.

Code now simplified ... On closer inspection, I conclude that the 'coord_name' of a _CoordConstraint is only ever intended to be a string. The only way it can be otherwise is if you construct Constraint(coord_values={something:value}). But whereas my earlier code suggested that 'something' could be a CoordDefn (or, now, a metadata), it actually can't since that is not a hashable type, so cannot act as a dict key. It is still just about "possible" that you could pass a Coord instance as the identifier in this case, but we have no tests for it.
I simply don't believe it is useful, so I'm no longer supporting it as a special case.

@trexfeathers I think this is good to go. Stephen's comment with change request is outstanding, but I think you can dismiss that if you think it is now OK.

Thanks @pp-mo! I'll let @stephenworsley take it over the line.

Thanks @stephenworsley I hope this latest addresses all the points raised. Sorry for the delay !

Thanks @stephenworsley I think I've now fixed them all to be the same.

I forgot to mention before, but this still needs a whatsnew entry.

Whoops sorry forgot. Doing it now ...

Hang on now I need to rebase/resolve...

Ok, rebased with added whatsnew

• please see if that suits, @stephenworsley

Thanks @stephenworsley for going the distance !

Thanks guys! 😀