Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fix character array handling in VOTable parsing and serialization (issue #17098)#19974

Open
cosmoJFH wants to merge 2 commits into
astropy:mainastropy/astropy:mainfrom
cosmoJFH:Include_char_arraycosmoJFH/astropy:Include_char_arrayCopy head branch name to clipboard
Open

Fix character array handling in VOTable parsing and serialization (issue #17098)#19974
cosmoJFH wants to merge 2 commits into
astropy:mainastropy/astropy:mainfrom
cosmoJFH:Include_char_arraycosmoJFH/astropy:Include_char_arrayCopy head branch name to clipboard

Conversation

@cosmoJFH

@cosmoJFH cosmoJFH commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Fix handling of character array fields, fix #17098

The current implementation does not correctly handle certain character array cases during VOTable parsing and serialization, leading to inconsistent behavior and failures in the scenarios described in the issue. This PR updates the character array handling to correctly process these cases

The same underlying issue has also been observed downstream in astropy/astroquery#3599, (Gaia and Euclid).

VOTables produced for the Gaia and Euclid missions are generated using the STILTS library, which supports the creation of VOTables containing string arrays.

The following is an example that illustrates the issue:

  1. Consider the following votable, where col2 defines an array of four elements, each consisting of 10 characters:
<?xml version="1.0" encoding="UTF-8"?>
<VOTABLE version="1.4" xmlns="http://www.ivoa.net/xml/VOTable/v1.3">
    <RESOURCE name="myFavouriteGalaxies">
        <COOSYS ID="sys" equinox="J2000" epoch="J2000" system="eq_FK5"/>
        <TABLE name="results">
            <DESCRIPTION>Velocities and Distance estimations</DESCRIPTION>
            <FIELD name="col1" ID="col1" datatype="char" arraysize="10*" unit="Mpc">
            </FIELD>
            <FIELD name="col2" ID="col2" datatype="char" arraysize="10x4">
            </FIELD>
            <DATA>
                <TABLEDATA>
                    <TR>
                        <TD>  cdefgh  </TD>
                        <TD>1234567890qwertyuiop0987654321abcdefghij</TD>
                    </TR>
                </TABLEDATA>
            </DATA>
        </TABLE>
    </RESOURCE>
</VOTABLE>
  1. When we try to open it, the following error is raised:
import astropy
from astropy.io.votable import parse

print(astropy.__version__)
6.1.7

votable = parse("votable_test.vot")


Traceback (most recent call last):
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/converters.py", line 328, in __init__
    self.arraysize = int(field.arraysize)
ValueError: invalid literal for int() with base 10: '10x4'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/table.py", line 164, in parse
    return tree.VOTableFile(config=config, pos=(1, 1)).parse(iterator, config)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 4202, in parse
    tag_mapping.get(tag, self._add_unknown_tag)(
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 4082, in _add_resource
    resource.parse(self, iterator, config)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 3871, in parse
    tag_mapping.get(tag, self._add_unknown_tag)(
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 3813, in _add_table
    table.parse(iterator, config)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 2707, in parse
    tag_mapping.get(tag, self._add_unknown_tag)(
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 2623, in _add_field
    field = Field(self._votable, config=config, pos=pos, **data)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 1386, in __init__
    self._setup(config, pos)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/tree.py", line 1429, in _setup
    self.converter = converters.get_converter(self, config, pos)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/converters.py", line 1316, in get_converter
    converter = cls(field, config, pos)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/converters.py", line 330, in __init__
    vo_raise(E01, (field.arraysize, "char", field.ID), config)
  File "/home/jfernandez/.local/lib/python3.10/site-packages/astropy/io/votable/exceptions.py", line 124, in vo_raise
    raise exception_class(args, config, pos)


astropy.io.votable.exceptions.E01: votable_test.vot:?:?: E01: Invalid size specifier '10x4' for a char field (in field 'col2')

Changes

We have included the following changes:

  • Introduced/refactored CharArray and CharArrayVarArray, along with their Unicode counterparts (UnicodeCharArray, UnicodeCharArrayVarArray), to unify and simplify character array handling.
  • Ensured compatibility with the existing public API.
  • Preserved existing binary and text serialization formats.

We have added regression tests covering the cases described in #17098. We have also included new tests:

  • Reproduce the failure on the current main branch.
  • Validate the corrected parsing and serialization behavior.
  • Cover both fixed-length and variable-length character array configurations where applicable.

This is our first contribution to Astropy, so we would welcome any feedback on the implementation approach or on the test coverage.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 2 times, most recently from 5a36756 to c4be280 Compare June 23, 2026 12:52
@cosmoJFH cosmoJFH changed the title Include char arrayFix character array handling in VOTable parsing and serialization (issue #17098) Fix character array handling in VOTable parsing and serialization (issue #17098) Jun 23, 2026
@pllim pllim added this to the v8.1.0 milestone Jun 23, 2026
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 2 times, most recently from 7db2f89 to d26fee6 Compare June 23, 2026 19:29
pllim

This comment was marked as resolved.

Sign in to view

@pllim

This comment was marked as resolved.

Sign in to view
@cosmoJFH

cosmoJFH commented Jun 24, 2026

Copy link
Copy Markdown
Author

Does this completely fix #17098 ?

Hi @pllim,

Yes, our intention is to fix #17098, as it is affecting the astroquery library, specifically the Euclid and Gaia modules (see astropy/astroquery#3599).

Please note that this is my first contribution to Astropy, so I would greatly appreciate any feedback.

Cheers,

Jorge.

@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 6 times, most recently from 27f5090 to 131bf83 Compare June 24, 2026 10:22
@pllim

This comment was marked as resolved.

Sign in to view
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 7 times, most recently from ee2e746 to bafd64e Compare June 25, 2026 09:06
@cosmoJFH

cosmoJFH commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Hi @pllim,

Please find my point-by-point responses below.

  • Please revert unnecessary style changes across the board; e.g., why remove blank lines, add weird indentations, and so on? There are A LOT of these.

The style changes were introduced automatically by PyCharm, which applies PEP 8 formatting rules. We can revert them if you would prefer to keep the existing style.

  • Do we really need 10 new data files? Can they be combined or tested in other ways, especially since you also added tests that do not need them.

We believe having separate files makes it easier to test and validate the different scenarios.

We developed new tests that read and parse VOTables in the BINARY2 format for the "unicodeChar" and "char data types.

The following files correspond to fixed-length arrays (10x4):

binary2_fix_length_array_char.xml
binary2_fix_length_array_unicodeChar.xml

The following files correspond to variable-length arrays (10x*):

binary2_variable_length_array_char.xml
binary2_variable_length_array_unicodeChar.xml

The following files cover higher-dimensional arrays, including fixed-length arrays (10x4x2) and variable-length arrays (10x4x*):

binary2_fix_length_multidimension_array_char.xml
binary2_fix_length_multidimension_array_unicodeChar.xml
binary2_variable_length_multidimension_array_char.xml
binary2_variable_length_multidimension_array_unicodeChar.xml

The following files are used by the new tests that read and parse regular VOTables (non-BINARY2 format) containing 1D string arrays:

valid_votable_with_char_array.xml
valid_votable_with_char_array2.xml
valid_votable_with_unicodeChar_array.xml
valid_votable_with_unicodeChar_array2.xml

The final two VOTables contain 2D string arrays:

valid_votable_with_char_multidimensional_array.xml
valid_votable_with_unicodeChar_multidimensional_array.xml

We also added the parameterized test test_table_write_char_arrays_verify, which exercises table.write() using different formats and the VOTables listed above.

Our testing shows that table.write() works correctly for fixed-length arrays of type "char". However, this is not the case for "unicodeChar" or multidimensional arrays. We are unsure whether support for those cases should also be included within the scope of this PR.

  • Need 2 change logs, one for io.ascii and one for io.votable

Could you give us more details about this?

Cheers,

Jorge

@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch from d15df54 to 3e32d1f Compare June 25, 2026 11:35
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 16 times, most recently from c879666 to 79eaad2 Compare July 3, 2026 12:47
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch from 5146fe8 to fb179ed Compare July 4, 2026 18:18
@bsipocz bsipocz added the Bug label Jul 7, 2026
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 7 times, most recently from 6130da8 to cca17ec Compare July 15, 2026 07:37
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch 2 times, most recently from 25b1687 to 021ebdc Compare July 21, 2026 06:34
@cosmoJFH
cosmoJFH force-pushed the Include_char_array branch from 021ebdc to 2e41769 Compare July 26, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim left review comments
@dhomeier dhomeier Awaiting requested review from dhomeier
@olebole olebole Awaiting requested review from olebole
@tomdonaldson tomdonaldson Awaiting requested review from tomdonaldson
@taldcroft taldcroft Awaiting requested review from taldcroft

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Bug io.ascii io.votable

Projects

None yet

Milestone

v8.1.0

Development

Successfully merging this pull request may close these issues.

VOTable reader does not accept array-like PARAMs from Topcat

4 participants

@cosmoJFH @pllim @taldcroft @bsipocz
Morty Proxy This is a proxified and sanitized view of the page, visit original site.