Skip to content
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

Problems inheriting from gridData's Grid class #56

Closed
AstroMike opened this issue Aug 24, 2018 · 3 comments
Closed

Problems inheriting from gridData's Grid class #56

AstroMike opened this issue Aug 24, 2018 · 3 comments
Assignees
Labels
Milestone

Comments

@AstroMike
Copy link

AstroMike commented Aug 24, 2018

I was using Grid as a base class for a class (in cosmology). For now let's call this CosmoGrid. But I ran into some issues.

First, the arithmetic operations (add etc) return Grid. I am not a OOP ninja, but I think if you have them return self.__class__(...) instead, then they will return an instance of the same subclass (CosmoGrid).

Second, one might want to wish to pass into Grid, not an ndarray but a subclass of an ndarray: for example an array of "Quantities" (that have astropy.units units attached, or a masked array). Will not the line self.grid = numpy.asarray(grid) convert this into a normal ndarray? (the docs say: Returns out: ... If a is a subclass of ndarray, a base class ndarray is returned.) The alternative might be numpy.asanyarray, for which the docs say "Convert the input to an ndarray, but pass ndarray subclasses through."

@richardjgowers
Copy link
Member

Hello @AstroMike !

re 1) yes OOP says that you should use self.__class__ to avoid mangling subclassers, so this is a bug in the code

re 2) I think using np.asanyarray might fix this, sounds like another good fix

@orbeckst
Copy link
Member

@AstroMike a PR would be most welcome – developer bandwidth is a bit limited at the moment. We're currently mostly occupied with getting a 1.0 of MDAnalysis out. This will likely be of 0% importance to you ;-). But we're very happy to make GridDataFormats better/more correct/more useful to anyone who finds a use for it.

@orbeckst
Copy link
Member

orbeckst commented Apr 6, 2019

@richardjgowers do you want to knock this one out? I want to do a 0.4.1 today or tomorrow – there were a whole bunch of bug fixes that should have been pushed out long ago

@orbeckst orbeckst added this to the 0.4.1 milestone Apr 6, 2019
@orbeckst orbeckst self-assigned this Apr 6, 2019
orbeckst added a commit that referenced this issue Apr 6, 2019
- use suggestion by @AstroMike in #56
- add test for using a DerivedClass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants