specutils icon indicating copy to clipboard operation
specutils copied to clipboard

Implementing indexing on a SpectralRegion object (but first decide if we should!)

Open eteq opened this issue 4 years ago • 2 comments

This is a follow-on from #790 (specifically the thread https://github.com/astropy/specutils/pull/790#discussion_r591854737). #790 implemented spectral indexing of the form spectrum[lowwl:highwl], using SpectralRegion as the underlying implementation. But what you cannot currently do is spectrum[SpectralRegion(...)]. While it seems straightforward, @rosteen pointed out there's a subtlety around how manipulation.extract_region works at the edges vs how traditional python indexing works. So it's not clear what's the right way of interpreting the edges in the case of spectrum[SpectralRegion(...)].

This may mean this feature shouldn't get implemented at all, because we should just tell users to either use extract_region or "normal" indexing and not something sort of in-between. But it seems weird to me that the implementation is so close to spectrum[SpectralRegion(...)] but we can't do that directly:shrug:

eteq avatar Mar 15 '21 13:03 eteq

I can't really think of any precent in the python ecosystem for passing python object instances to the indexing machinery that's not actually some kind of slice. It feels so dirty! Why not just spectrum.from_region(SpectralRegion(...)), trying to make the indexing formalism work like a function call is really strange.

nmearl avatar Mar 15 '21 16:03 nmearl

This has been open for a while - I agree with @nmearl that this is going too far with the slicing machinery, and that we should have the users take advantage of the existing extraction tools for SpectralRegion. Can we close this as a "won't implement"?

rosteen avatar Feb 08 '22 19:02 rosteen