Changed to auto convert from aux to dim coord

[niallrobinson]

Currently iris raises an error if you try and pass an aux coord to a function that wants a dim coord, even if the aux coord is capable of being converted to a dim coord (i.e. monotonic). This PR attempts to convert aux coords and then raises the current error if that fails.

Note I haven't updated the documentation - not sure what to say: "or an aux_coord that can be converted using DimCoord.from_cube()" or something?

[ajdawson]

This is considered bad practice as it will catch any exception at all, including things like keyboard interrupts, which are outside the desired behaviour. You should have some idea of what is likely to go wrong and catch only those errors.

[ajdawson]

I've made a couple of minor coding comments. However, I'm not confident in deciding if this is generally a good idea or not so I'd like to let others provide feedback too (although the idea seems fairly reasonable to me).

[niallrobinson]

if this is generally a good idea

I know what you mean - I'm just (as usual) pushing from the user side - its something that I find myself doing a lot, and I can't think of a reason why I should...doesn't mean there isn't any!

[niallrobinson]

Changes made

[ajdawson]

What extra information is contained in str(e) (not necessarily a criticism, I'm at a windows machine so I can't check myself)?

If it doesn't add much useful I'd say ditch it and go for a more descriptive static message. On the flip side, if e already contains a descriptive message that is good to use as is then we don't even necessarily have to re-raise it...

[niallrobinson]

ValueError: Could not convert coord to DimCoord: The points array must be strictly monotonic.
Probably worth keeping, do you think? Could do with being changed from a colon after DimCoord thought

[ajdawson]

OK, then I guess what I want to know is will it ever be anything other than that? If not then you could just write that... If it can be one of any number of things then maybe this is OK.

[rhattersley]

Interesting.

I'll try to give this some thought on Monday, and check for performance implications, etc.

[niallrobinson]

Is travis unwell? I managed to restart this but it failed at intsalling pyke

[pp-mo]

Is travis unwell?

Probably.
My latest survived installing Pyke, but fails at the 'unpack' library.
(mine : #1040)
Yours says "timed out", while mine says "connection refused". But it could still be a general slowness/timeout problem I guess.

I'll restart mine + see if it fails in the same way.

[esc24]

I object to this. Currently if I add a coordinate to a cube, the reference to the object that I have is a reference to the coordinate that is now in the cube. The result of this PR is to break that relationship some, but not all, of the time. Implicit changing of types is in my view undesirable and potentiality confusing behaviour.
I can completely understand why the current functionality is frustrating, I should be able to add any coordinate-like object that is 1d, numeric, and strictly monotonic, but the problem is you can change the values of points in a AuxCoord throughout its lifetime, so while it may have met the criteria initially, we cannot be sure it will continue to. Your solution of changing types implicitly will work, but it's opaque in my view, and has the potential to break existing code that relies on coord is cube.coord(...).

[niallrobinson]

Perhaps I haven't understood the full implications of what I've done then.

I thought I was creating a completely new and separate dim coord from the aux coord instance. If I understand you correctly, I am creating a new version of the aux_coord which is masquerading as a dim_coord? i.e. if the original aux_coord is changed it will break anything that uses its dim_coord version? Or have I go the wrong end of the stick?

but the problem is you can change the values of points in a AuxCoord throughout its lifetime

Would you be any happier if we spawned a new instance of the coord?

The fundamental problem is the part of the definition of dim_coord as having to be something defining a cube's dimension. In this context its irrelevant weather the coord was used to define a dim, or as a secondary coord as long as its monotonic etc. However we currently still make that distinction with this check. I fully admit that this PR might not be the answer to that conundrum though.

[ajdawson]

I thought I was creating a completely new and separate dim coord from the aux coord instance.

You are, this is the problem as highlighted by @esc24.

If I understand you correctly, I am creating a new version of the aux_coord which is masquerading as a dim_coord? i.e. if the original aux_coord is changed it will break anything that uses its dim_coord version? Or have I go the wrong end of the stick?

The other way around. Currently the coordinate on the cube and the instance that was added are the same thing, so if you change one you change the other. Your modification breaks this by using an entirely new DimCoord instance.

[niallrobinson]

literally 180 deg the wrong end of the stick - ok, I'm with you now.

I still think there might be problem in that the coords are defined by how they are used, not what properties they posses

[esc24]

I still think there might be problem in that the coords are defined by how they are used, not what properties they posses

I'm not sure I agree. DimCoords can always be relied upon to be numeric, 1d, strictly monotonic coords, AuxCoords do not have those constraints. I see these as properties. As for the containers cube.dim_coords and cube.aux_coords, there is confusion given the latter can contain both types. Could you explain what you mean by "the coords are defined by how they are used"? Perhaps an example would help me understand.

[niallrobinson]

An aux coord and a dim coord can be identical in all but name. The fact that the from_coord method can exist at all suggests the coords are (or should I say "can be") defined by how they are used i.e. weather they are used as the primary description of a dimension or not. I think it would be better if we had:

1. coord which is numeric, 1d, strictly monotonic
2. coord which isn't all of (number, 1d and strictly monotonic)

infact, conceptually, there are three types of coord in a cube

1. coord which is numeric, 1d, strictly monotonic and used as the primary dimension coordinate
2. coord which is numeric, 1d, strictly monotonic but not used as the primary dimension coordinate
3. coord which isn't all of (number, 1d and strictly monotonic) and can't be used as the primary dimension coordinate

This comes up when trying to use aux coords from one cube to construct a new cube for instance. Its really not a big deal adding one more line of code, so a involved redesign might not be worth it, but I do think that the current solution is sub-optimal. However, I think that one line is asking the users to navigate round the design of iris. I remember finding the distinction between aux and dim very confusing when I first started using iris

[niallrobinson]

Have we run out of steam on this one? I suspect the consensus might be "you might have a point but it needs a lot more coding to sort a relatively small issue"?

Bottom line is that I think DimCoord.from_coord() exists for users to navigate around the iris api, rather to do something that is conceptually a task that a user wants to perform

[ajdawson]

Have we run out of steam on this one?

I think so. It was a reasonable enough idea, but unfortunately it breaks the existing API in that we could no longer guarantee the relationship between a particular coordinate object and its corresponding coordinate on a cube. This may seem minor but I think it is quite a big deal in terms of the design and how much user code it could end up breaking. If you can agree with that point perhaps you could close the PR @niallrobinson?

Your other point is much more philosophical in nature and would warrant discussion in a wider forum if you still want to pursue it. I doubt anyone will drop by the PR to discuss it further anyway.

[niallrobinson]

Your other point is much more philosophical in nature

yeh - it escalated fast!