Repository navigation
Allow additional kwargs when reading VOTables using the Unified I/O - #11643
Conversation
The file 'astropy/io/votable/tests/table_test.py' contained multiple functions that imported Table and Column from astropy.table. All those import statements are now replaced with a single line placed among the other import statements.
When reading in VOTables with the `astropy.io.votable.table.parse()` function it is possible to specify many keyword arguments, and there is no obvious reason why those arguments should not be usable through the Unified File Read/Write Interface. This commit adds a test to check that keyword arguments meant for `parse()` can be specified using 'Table.read()'.
If a VOTable is read using the Unified File Read/Write Interface, the function `astropy.io.votable.table.parse()` get called through the function `read_table_votable()` in 'astropy/io/votable/connect.py'. Both the Unified I/O Interface and `parse()` can be called with many useful keyword arguments. This commit enables `read_table_votable()` to pass those keyword arguments from one to the other.
|
The changes introduced in this pull request are simple enough and the functionality of the code intuitive enough that I am not sure if this would be a new feature or a bug fix. I will create the change log entry once a milestone has been assigned. |
|
I think it is more of a feature than bug fix, since it changes the API? I wouldn't backport this, personally. We have changed the way we do change log, so you don't need to know the milestone first now. See https://github.com/astropy/astropy/blob/main/docs/changes/README.rst . Thanks! |
tomdonaldson
left a comment
tomdonaldson
left a comment
There was a problem hiding this comment.
@eerovaher Thanks for adding this behavior. It's definitely useful!
The changes look good to me.
|
Yes, let's do that.
Write your change log entry in a file named
Yes. Please use the |
| from astropy.table import Column, Table | ||
| from astropy.units import UnrecognizedUnit |
There was a problem hiding this comment.
I wonder why these weren't top level before. Perhaps we used to have circular import problem? Anyways, CI passed, so this should probably be okay... 🤞
| # Table.read() should pass on keyword arguments meant for parse() | ||
| filename = get_pkg_data_filename('data/nonstandard_units.xml') | ||
| t = Table.read(filename, format='votable', unit_format='generic') | ||
| assert not isinstance(t['Flux1'].unit, UnrecognizedUnit) |
There was a problem hiding this comment.
Isn't it more explicit to test that the unit is actually what you want it to be, rather than simply not UnrecognizedUnit?
There was a problem hiding this comment.
I have updated the test.
pllim
left a comment
pllim
left a comment
There was a problem hiding this comment.
I left some minor comments. I will approve after they are addressed and change log is added.
If you change the code as suggested, let's let the CI run again rather than skipping.
Thank you!
|
Yes, you can create a second fragment file, but in this case I think |
Description
Reading in VOTables with the functions
astropy/astropy/io/votable/table.py
Line 170 in 8df6dcf
astropy/astropy/io/votable/table.py
Line 30 in 8df6dcf
allows the user to specify many keyword arguments, which are not available when using the Unified File Read/Write Interface. This pull request would allow all keyword arguments that are valid for the
parse()function to be specified when calling theTable.read()orQTable.read()functions.An example of where this would be useful is reading in Gaia archive VOTables as QTables. Calling something like
QTable.read(filename, unit_format='vounit')would be enough to ensure that composite units get parsed properly.Fixes #10791