from_header used improperly?

[keflavich]

I may be completely off, but I think there's a major issue in AVM.__init__:

These lines:

            if type(origin) is str:
                self.from_file(origin)
            elif astropy_installed and isinstance(origin, fits.Header):
                self.from_header(origin)
            elif astropy_installed and isinstance(origin, WCS):
                self.from_wcs(origin)

all call functions that, inside, define self = (something that generates an AVM instance). However, they are simply reassigning the local variable self to be a new AVM instances, so the self that is the AVM instance we're trying to build never has any of the metadata.

So you can do this: avm = AVM.from_header(header) but this doesn't work: avm = AVM(header)

Now, I don't claim to fully understand classmethods yet, but my testing by using print statements & pdb showed that the AVM instance generated in the latter method never received any of the metadata (specifically, Spatial).

[astrofrog]

Ah yes I just switched to classmethods, so this needs fixing - thanks!

[astrofrog]

I need to think a bit more about how to properly initialize the specs version now that I use classmethods. I'll likely fix it this evening.

[astrofrog]

In the latest version of master, I've actually decided to remove the initialization of the form AVM(something) because there's now quite a lot of ways to initialize the object, so I'd rather the class methods be used. Of course, this is backward-incompatible, but the next release will be considered the first 'proper' release, because there was a lot broken before anyway (so APLpy will require the newest release for example).

[keflavich]

OK, that sounds good - I think that means my APLpy PR makes sense.

If you DID want to keep the AVM(something) initialization, is there a way to do it with the classmethods you have? All I could think of doing was:

newobj = self.from_header(header)
for key in list_of_keys_to_copy:
    self.__dict__[key] = newobj.__dict__[key]

but that certainly seems kludgy and ugly, so I like your choice to remove it.

[astrofrog]

Yeah, it wasn't obvious to me how to do it cleanly... I'll go and review the PR for APLpy now!