spacetelescope/synphot_refactor

Add a 'name' attribute to Spectrum instances

ibusko opened this issue · 10 comments

In pysynphot, spparser.parse_spec() returns an object that has a name attribute. This is no longer true in synphot.

We request that the same behavior be maintained in synphot.

pllim commented

Can you please provide an example, so I can make sure we're both on the same page? Thanks!

pllim commented
>>> import pysynphot as S                                                   
>>> sp_old = S.parse_spec('bb(5000)')                                       
>>> sp_old.name                                                             
'BB(T=5000)'
>>> import stsynphot as stsyn                                               
>>> sp_new = stsyn.parse_spec('bb(5000)')                                   
>>> sp_new.meta['expr']                                                     
'bb(5000.0)'

The reason it is in meta now is to be consistent with astropy.modeling.models data structure. It is under a key named 'expr' because PySynphot uses that key in its FITS I/O.

>>> from astropy.modeling import models                                     
>>> g = models.Gaussian1D()                                                
>>> g.meta                                                                 
OrderedDict()

If you really need the .name attribute, perhaps implement it in your subclass and alias it to sp.meta['expr']? Will that work for you?

pllim commented

To clarify further, this is set in the model level. For blackbody, it would be:

self.meta['expr'] = 'bb({0})'.format(self.temperature.value)

The point is that the expression in the meta directory is not very helpful in many cases, such as

>>> import stsynphot as stsyn
>>> sp = stsyn.parse_spec('rn(unit(1.,flam),band(johnson,v),15,vegamag)')
>>> sp.meta['expr']
"rn(<synphot.spectrum.SourceSpectrum object at 0xb1cbb3710>, <stsynphot.spectrum.ObservationSpectralElement object at 0xb1c821550>, 15.0, 'vegamag')"
pllim commented

If your intended usage is solely for parse_spec, the expr for the rn() call can be fixed in stsynphot.

I am a little concerned about adding the name attribute in a generic way in synphot because a concise name can only be given to a few basic models and to those we explicitly define in this package. The goal of using Astropy models is that anyone can pass in their own models and they can be compound models, then the name attribute will quickly lose its meaning and end up confusing users.

pllim commented

Update: I am able to fix the .meta['expr'] values over at stsynphot except for something that used the parser operator like this:

'rn(crcalspec$bd_75d325_stis_002.fits, band(u), 9.5, vegamag) * band(fos, blue, 4.3, g160l)'  

I think this needs to go into __mul__ operator in SourceSpectrum. But it also opens a can of worms, because I will have to also implement it for other operators and handle different combinations. Do you really need a human-readable name for this particular IRAF string in ETC?

I think that the idea for this kind of string is to display it as is. That is, if the expression, as a string, can be stored in the observation instance in an easily retrievable way, as a string, that would suffice. One can make it simpler by applying the same concept to all strings. That is, even if a particular element of the spectrum expression, such as bb, could be conceivably translated to stringBlack body, leaving it as bb wouldn't be a problem. In fact, that's the way the pysynphot-based pyetc works.

pllim commented

The idea is one thing. But if in reality, ETC never uses parse_spec for 'something * something' and try to display the name of that object, then I don't have to implement it. parse_spec is more of a legacy usage as it is not Pythonic.

pllim commented

Update: I made a stsynphot 0.2.2 release to fix the clarity of .meta['expr'] content in parse_spec, in addition to fixing a bug (spacetelescope/stsynphot_refactor#103).

If this is good enough for your use case, please close this issue. Otherwise, please let me know what else is missing that you absolutely must have. Thank you!

@pllim thanks!