LSSTDESC / sacc

Save All Correlations and Covariances
BSD 3-Clause "New" or "Revised" License
15 stars 8 forks source link

FITS table for each tracer is duplicated #86

Closed marcpaterno closed 1 year ago

marcpaterno commented 1 year ago

The method BaseTracer::__init_subclass__ puts two entries into the class variable _tracer_classes when a subclass that inherits from BaseTracer has a tracer_type string with an upper-case character. This results in the generated FITS file containing two identical array entries, each corresponding to this tracer.

Running the Create_Sacc.ipynb notebook in the sacc/examples directory shows this effect.

It appears this was added in commit e9e302408844d013a3e92b2a50f9dc33bfd4caa9. It looks like the reason for this commit was to allow reading old files, presumably ones that had the tracer_type specified in lower case.

It appears this might be fixed by (1) removing the code that adds the duplicated entry into the dictionary and (2) altering the implementation of BaseTracer.from_tables to deal with lower-case names in old files.

Or perhaps this “old files” are now so old that nobody needs to support them, and the second change is no longer needed.

damonge commented 1 year ago

I'd vote for just assuming that we don't need to support old files. @joezuntz , are you guys using those in TXPipe? I think you may be the only consumers of those.

joezuntz commented 1 year ago

That was probably the reason, though I don't remember. Agree with @damonge that we can stop supporting those files if they are around and fix the code making them instead. So please do change this if you wish.