Remove 'iris.analysis.coord_comparison' from public API. #3562
Conversation
157396c
to
b455d24
Compare
|
Thanks @stephenworsley . |
|
@pp-mo Looks good now, I reckon this is good to merge. |
| @@ -186,16 +185,53 @@ def matches_any(self, predicate): | |||
| return any(self.matches(predicate)) | |||
|
|
|||
|
|
|||
| def coord_comparison(*cubes, object_get=None): | |||
| def _dimensional_metadata_comparison(*cubes, object_get=None): | |||
There was a problem hiding this comment.
@pp-mo Awesome! 👍
My only suggestion here, before we commit to this, is questioning the name of this private function (all 30+ characters of it).
I'm just making the explicit point that it can be anything that we want - it's private. I'm all for meaningful, appropriate names, rather than those that are cryptic, abstract or obfuscated, but now is probably the time to suggest shorter alternatives? It is a bit of a mouthful, right? 😮
Any suggestions? Can I start the bun-fight with simply _metadata_compare?
There was a problem hiding this comment.
@bjlittle questioning the name of this private function
We just missed the boat on this.
I get the point, but I couldn't think of anything more concise and still clear.
Probably the term DimensionalMetadata is at fault here, also being a bit of a mouthful.
I guess we could have _dm_comparison.
Do you want to propose a post-fix in separate issue ?
There was a problem hiding this comment.
@pp-mo @stephenworsley No worries. Naming a thing is the hardest part, strangely.
I recently asked @lbdreyer for an alternative name to _DimensionalMetadata.... but we came up blank 😕
This isn't a high priority, by any means, just wanted to raise it as an obvious point to consider. We can stick with what we've got, and change at a later date in a follow-up PR, if we have an epiphany 💡 with agreement
Replaces #3558
Following discussions there, I'm proposing to remove the original, public 'coord_comparison' function. Replaced with an augmented, private version for internal use only.
In offline discussions with @bjlittle @lbdreyer , we agreed this was always rather over-complicated and under-explained for a user public method. Plus, the key
_CoordGroupclass it relies on was always private anyway.Since it was practically impossible to understand or use with confidence, without also reading the sourcecode, we decided to simply retire it rather than providing an enhanced public function (or functions).