-
Notifications
You must be signed in to change notification settings - Fork 284
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
Minimise the cube API #3429
Comments
NOTE: I've labelled this for Iris 3.0, because it suggests a serious breaking change. |
😨 I think I can see where you're coming from, but from a pure user perspective this might just look like "making us change our code for no good reason". I note that the comment you link specifically says that removing the One big difference between the numpy and iris APIs is that all the numpy function are available in the top level package, so I am only ever 3 keystrokes ( |
Note that is has also previously been suggested that there is too much in |
TBH this is all about making the code tidier, which is basically for developers, not users ! A counter-example though is how hard it is to navigate the iris.cube docspage |
Though, re: I do enjoy a good multilevel hierachy, don't you ? 😉 re:
But that is really the same problem + I think supports my case, because that is just what's wrong with |
I couldn’t possibly comment 😆 I think it’s fair to say that a lot of functionality is not where you would put it if you were starting from scratch. But if we’re going to move things around, we need to consider that an awful lot of code has been written on top of Iris, so it should be as easy as possible to update said code to work with new versions. Perhaps consider creating tools to automate this? Like how |
Or we could borrow the time machine from the numpy folks.... |
BTW, are we still doing the deprecation thing? https://scitools.org.uk/iris/docs/latest/developers_guide/deprecations.html |
Well, I still think so. For specific instance : did this issue today introduce a breaking change, |
that link isn't working for me 😕 |
Sorry, I fixed it, meant to link #3431. The form |
😁 |
Can I just raise that this is a perfectly valid way to define a method: def collapsed(self, coords, aggregator, **kwargs):
pass # actual contents of Cube.collapsed here...
class Cube:
collapsed = collapsed In fact, so is this (unless you've done something particularly egregious with metaclasses): class Cube:
pass
Cube.collapsed = collapsed Basically there's nothing special about a I think moving functions away from the class definition and into dedicated modules is a good idea regardless, 4000 lines is far too much for one file. So really I'd say the choices are:
|
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
In my opinion, there is far too much in
cube.py
, and particularly the Cube class.I would like to see this minimised by removing purely "convenience" instance methods : By this, I mean things which don't rely (?much) on private class implementation details, so they could just as well be cast as independent operations, which would take the cube as an argument.
As 'utility' functions, these would then exist at the 'cube' package level or even in the generic
iris.utils
package.In my mind, this means making a distinction between core operations which define what a cube "is" and are heavily concerned with the implementation, like
add_dim_coord
and__getitem__
, and those which are secondary operations that just "use" cubes.Certainly, if an operation can be rewritten to work entirely via the public Cube API, then it does not belong in the class. Surprisingly, even functions like 'transpose' may fall into this category.
Specifically, I think all of the following could potentially be moved outside of the Cube class:
There is another set which could also be addressed, but which are currently in-place operations (see also #3430). If they were rewritten as functions which always return a new cube, then these could also be "taken out" :
There are some interesting parallels here with the numpy API.
For instance you could remove all those array instance methods, like array.astype, array.reshape, array.transpose etc, when you can instead just use np.astype(array, X) etc.
Most of those do simply redirect to static methods.
I have also seen a comment on the numpy project that they "wish" they had reduced the proliferation of unnecessary instance methods, but it is now too late !
The text was updated successfully, but these errors were encountered: