Open jacob-ai-bot[bot] opened 4 weeks ago
Hello human! 👋
This PR was created by JACoB to address the issue Incorrect units read from MRT (CDS format) files with astropy.table
Please review the PR carefully. Auto-generated code can and will contain subtle bugs and mistakes.
If you identify code that needs to be changed, please reject the PR with a specific reason. Be as detailed as possible in your comments. JACoB will take these comments, make changes to the code and push up changes. Please note that this process will take a few minutes.
Once the code looks good, approve the PR and merge the code.
Summary:
Description
When reading MRT files (formatted according to the CDS standard which is also the format recommended by AAS/ApJ) with
format='ascii.cds'
, astropy.table incorrectly parses composite units. According to CDS standard the units should be SI without spaces (http://vizier.u-strasbg.fr/doc/catstd-3.2.htx). Thus a unit oferg/AA/s/kpc^2
(surface brightness for a continuum measurement) should be written as10+3J/m/s/kpc2
.When I use these types of composite units with the ascii.cds reader the units do not come out correct. Specifically the order of the division seems to be jumbled.
Expected behavior
The units in the resulting Table should be the same as in the input MRT file.
How to Reproduce
Get astropy package from pip
Using the following MRT as input:
And then reading the table I get:
For the SBCONT column the second is in the wrong place, and for SBLINE kpc2 is in the wrong place.
Versions
@jacob-ai-bot --skip-build --branch issue_14355 https://github.com/astropy/astropy/issues/14355
Plan:
Step 1: Edit
/astropy/units/format/cds.py
Task: Fix CDS parser to handle multiple divisions correctly
Instructions: Modify the CDS unit parser in
/astropy/units/format/cds.py
to correctly handle composite units with multiple divisions. The parser should interpret consecutive divisions in the correct order, ensuring that units like '10+3J/m/s/kpc2' are parsed asJ/(m*s*kpc^2)
or an equivalent representation that reflects the intended order of operations. Address any issues that might arise from incorrect grouping or application of powers during parsing, specifically focusing on how the composite units are decomposed and represented in the parser's grammar. Verify that the parsed unit strings result in the correct unit objects when used in conjunction with theastropy.units.Unit
class. Consider adding explicit tests within/astropy/units/tests/test_format.py
or a new dedicated test file under/astropy/units/tests/
to cover the corrected parsing behavior with examples involving multiple divisions. These tests should include various composite units with multiple divisions, including edge cases, to confirm that the fix does not introduce regressions.Exit Criteria: The
astropy.units.format.CDS.parse
method should correctly parse composite units with multiple divisions, such as '10+3J/m/s/kpc2', resulting in units equivalent toJ/(m*s*kpc^2)
. Unit tests should confirm the correct parsing behavior and the absence of regressions for other unit combinations.Step 2: Edit
/astropy/units/core.py
Task: Ensure consistent unit order during conversion and representation
Instructions: Modify the unit conversion and representation logic in
/astropy/units/core.py
,/astropy/units/format/cds.py
, and/astropy/units/format/generic.py
to consistently preserve the intended order of units during conversion, especially in cases involving multiple divisions. Ensure that the units in the resultingastropy.table.Table
object match the order in which they are presented in the input MRT file. Update the_get_converter
method in/astropy/units/core.py
to maintain the correct unit order during conversions with multiple divisions, and adjust theto_string
method in both/astropy/units/format/cds.py
and/astropy/units/format/generic.py
to reflect the intended order in string representations. Add tests in/astropy/units/tests/test_format.py
or a dedicated test file under/astropy/units/tests/
that check that unit order is preserved after conversion and that the units in the converted table match the original MRT file. These tests should include various unit combinations with multiple divisions to validate the fix thoroughly.Exit Criteria: Unit conversions involving composite units with multiple divisions should maintain the order specified in the input. The resulting units, both as objects and in string representations, should match the intended order. Unit tests should confirm the correct behavior across different scenarios.