Open jacob-ai-bot[bot] opened 4 weeks ago
Hello human! 👋
This PR was created by JACoB to address the issue ascii.qdp Table format assumes QDP commands are upper case
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
ascii.qdp assumes that commands in a QDP file are upper case, for example, for errors they must be "READ SERR 1 2" whereas QDP itself is not case sensitive and case use "read serr 1 2".
As many QDP files are created by hand, the expectation that all commands be all-caps should be removed.
Expected behavior
The following qdp file should read into a
Table
with errors, rather than crashing.How to Reproduce
Create a QDP file:
Running "qdp test.qdp" works just fine.
Versions
Python 3.10.9 (main, Dec 7 2022, 02:03:23) [Clang 13.0.0 (clang-1300.0.29.30)] astropy 5.1 Numpy 1.24.1 pyerfa 2.0.0.1 Scipy 1.10.0 Matplotlib 3.6.3
@jacob-ai-bot --skip-build --branch issue_14363 https://github.com/astropy/astropy/issues/14363
Plan:
Step 1: Edit
/astropy/io/ascii/qdp.py
Task: Modify QDP command parsing to be case-insensitive
Instructions: In the
_line_type
function within the/astropy/io/ascii/qdp.py
file, modify the regular expression_command_re
to handle the 'READ' command in a case-insensitive manner. The current implementation,r"READ [TS]ERR(\s+[0-9]+)+"
, explicitly requires "READ" to be uppercase. Update the regex to allow for lowercase variations of the "READ" command. The rest of the command, including "SERR" or "TERR", should already be case-insensitive due to the use ofre.IGNORECASE
when compiling the regular expression. Consider potential conflicts with data lines that might inadvertently be interpreted as commands due to this change.Exit Criteria: The
_line_type
function should correctly identify the "READ SERR" and "READ TERR" commands regardless of the casing used for "READ". The function should also correctly differentiate between valid commands and data lines that might resemble commands due to the case change. Running the example provided in the issue description should now succeed without raising a ValueError.Step 2: Edit
/astropy/io/ascii/tests/test_qdp.py
Task: Update QDP roundtrip test case to include lower-case commands
Instructions: Within the
/astropy/io/ascii/tests/test_qdp.py
file, review thetest_roundtrip
function and its associatedexample_qdp
data. Ensure that this test data includes QDP command lines with lower-case variations of theREAD
command, such asread serr
,rEaD serr
, etc., to test the case-insensitive parsing implemented in the previous step. The existing parameterized test already handles lowercasing the header, but it's crucial to ensure that the test data itself covers these variations. If theexample_qdp
data does not already contain lower-case commands, add them to provide more comprehensive test coverage. Verify that the roundtrip functionality continues to operate as expected with the updated test data.Exit Criteria: The
test_roundtrip
function should pass successfully with the updatedexample_qdp
data, demonstrating that the QDP reader and writer correctly handle command lines regardless of the case used forREAD
.