Repository navigation
Adding the conversion of dex(xxx) units to fits format (i.e. log(xxx)) #17297
Description
What is the problem this feature will solve?
Currently, the FITS format of the dex units of Astropy is not in accordance with FITS standard, and Astropy cannot read the log units in FITS format. This has been actually suggested in #3822.
For example, current outcomes are:
>>> u.dex(u.Msun).to_string(format='fits')
'dex(solMass)'>>> u.Unit("log(solMass)", format="fits")
ValueError: 'log(solMass)' did not parse as fits unit: 'log' is not a recognized function If this is meant to be a custom unit, define it with 'u.def_unit'. To have it recognized inside a file reader or other code, enable it with 'u.add_enabled_units'. For details, see https://docs.astropy.org/en/latest/units/combining_and_defining.htmleven though log(solMass) is a perfectly legal FITS format unit.
Describe the desired outcome
The desired outcome would be:
>>> u.dex(u.Msun).to_string(format='fits')
'log(solMass)'>>> u.Unit("log(solMass)", format="fits")
dex(solMass)Additional context
For reference, in the FITS standard (Table 6), the counterpart to the Astropy dex(xxx) unit is log(xxx).
Activity
github-actions commentedon Nov 1, 2024
Welcome to Astropy 👋 and thank you for your first issue!
A project member will respond to you as soon as possible; in the meantime, please double-check the guidelines for submitting issues and make sure you've provided the requested details.
GitHub issues in the Astropy repository are used to track bug reports and feature requests; If your issue poses a question about how to use Astropy, please instead raise your question in the Astropy Discourse user forum and close this issue.
If you feel that this issue has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail [email protected].
LucasToscanou commentedon Nov 26, 2024
Hello there! Do you think I could try this one as a good second issue?
I saw a comment on #3822 that said that it shouldn't be hard someone just had to spend some time on it
mhvk commentedon Nov 27, 2024
@LucasToscanou - I don't think it is too tricky, but it does require going quite a bit into the internals of units and, to a lesser extent, fits. As I wrote in #3822, maybe start by writing some tests. Probably good to look up the FITS standard to see if they give some examples of how things should be interpreted.
CastleStar14654 commentedon Dec 12, 2024
It's great to know this issue is getting handled.
I checked a bit into the code. The string formating/conversion of FITS unit format is handled by class
Fitsinherited fromGeneric(https://github.com/astropy/astropy/blob/main/astropy/units/format/fits.py).I suppose that one easy approach might be check the
cdsformat parser, and imitate some of the codes there. (https://github.com/astropy/astropy/blob/main/astropy/units/format/cds.py)mhvk commentedon Dec 12, 2024
@CastleStar14654 - indeed, that approach would make sense, certainly for the writing-out part. For the parsing, FITS shares its parser with
generic.py, but there are already some exceptions in there to take care of small differences between FITS and generic units, so this would just mean adding another one.