Repository navigation
Read VOTable BINARY and BINARY2 tables a column at a time - #20828
Open
jdavies-st wants to merge 9 commits into
Open
jdavies-st wants to merge 9 commits into
jdavies-st wants to merge 9 commits into
Conversation
- When every column holds a single number, boolean, bit or string, read the rows 1 column at a time with numpy instead of one value at a time. - The results are the same as before: values, masks, dtypes, warnings and errors. Tables with other column types, data fetched from a URL, and rows that need a warning or error (such as a string over its length limit, or a bad byte) are still read value by value. - Strings in columns that are not requested are still checked, so a bad string raises the same error as before. - Add tests comparing the two readers on every supported datatype, and for a string over its limit, a partial last row, a bad string in a column not read, and an empty stream. - Indicate which tables are read faster in the VOTable performance docs, and add a changelog entry.
- Give each converter the column reader handles a method that converts a whole column of stored values, next to its binparse, so the two rules cannot drift apart: numbers mask their null value or NaN, booleans use the codes of the binparse mapping, bits test the bit that binary_one sets, and fixed-length strings are cut at their first NUL. - Decode each variable-length string straight from the stream, instead of building a byte mask of the whole chunk for every unicodeChar column. This is also faster for char columns. - Convert fixed-length char values by widening each byte to its code point, and cut at the first NUL only the values with other bytes after it. The running count of NULs this replaces took 8 times the memory of the column, and made tables with wide char columns, such as char[255], slower to read than value by value. - A bad byte in a char string now always stops the column reader the same way, with the decoding error, whether or not the column is requested. - Share the list of datatypes that the BINARY2 null bitmap never masks with the value-by-value reader.
- Any exception in the column reader now hands the stream to the value-by-value reader, which gives the warning or error, instead of ending the read. - Read char and unicodeChar fields of arraysize 0 as empty strings. The column reader raised for them before. - Read a table whose rows take up no bytes as no rows. The value-by-value reader never stops on such a table. - Hand a stream that ends partway through a row to the value-by-value reader, which decodes the values in that row and raises for a bad one. - Add a test for each.
- After reading every row by columns, warn W46 for each string over its arraysize limit, in the order the value-by-value reader gives, instead of reading the whole table again value by value. - Give the char and unicodeChar converters one method for the warning, used by both readers. - Test that both readers give the same warnings in the same order.
- Take the end of each row from the scan of the stream, and choose chunk boundaries from those ends, so a run of large rows no longer makes a chunk far bigger than 16 MiB. - Also limit a chunk to 65536 rows, which limits the per-row offsets and lists for tables of small rows. - Test chunks of one and two rows against the value-by-value reader, and the chunk boundaries for rows of mixed sizes.
- Check that BINARY tables use the column reader too, not only BINARY2. - Cover integer VALUES null and the BINARY2 null bitmap on int and bit columns.
- On the example tables of pyvo and astroquery, grown to 200,000 rows, the column reader is 4.6 times faster at the median, over 10 times for tables of numbers only, and about 2 times for tables made mostly of variable-length strings. - Update some code comments.
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.
|
- Improve the comments the column reader so they say what the code around them does not - Add PR number to changelog fragment
astrofrog
added
Extra CI
pllim
reviewed
Nov 11, 2026
| # Licensed under a 3-clause BSD style license - see LICENSE.rst | ||
| """ | ||
| Tests of reading a whole BINARY or BINARY2 stream a column at a time, in | ||
| `~astropy.io.votable.tree.TableElement._parse_binary_columns`. |
Member
There was a problem hiding this comment.
No need to use Sphinx ref here. Hidden function will never be exposed in API doc anyway. And we don't render test docstrings.
pllim
reviewed
Nov 11, 2026
Member
|
CI is in a weird state. Might be transient server issues. Let's see if it clears itself out on the next commit... |
Contributor
Author
|
Pasted from the benchmarks job:
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY. 🚀 |
Co-authored-by: P. L. Lim <[email protected]>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is the last of a small series of PRs on reading VOTable BINARY and BINARY2 data, after #20509, #20513 and #20743.
This PR is purely for performance.
Currently every value in a BINARY or BINARY2 stream is read one at a time in a Python loop that calls each field's converter in turn. With the Cython coverters added in #18454, that value-by-value read was sped up significantly. With this PR, the whole stream is read a column at a time using numpy. It does this for every column that is a scalar - number, boolean, bit or string (
charorunicodeChar, fixed or variable length). And only if the data is inline in the VOTable inside the STREAM element.A few notes on how it works:
_binparse_columnmethod (_binparse_var_columnfor variable-length strings) next to its regular binparse, which converts a whole column the way binparse converts one value. The new reader in tree.py only works out where each column's bytes are, and converts the rows in chunks so the temporary memory doesn't go nuts.charfield with arraysize="0") used to make the reader loop forever. It now gives an empty table.Besides the new unit tests, I compared the two readers on all the pyvo and astroquery test suite tables.
Downstream unit tests in pyvo and astroquery pass when run locally with the tox job in #20604.
I also ran the VOTable tests on a big-endian machine (s390x in Docker). Please turn those on in CI.
And could we also label so that the benchmark CI runs? Most of the VOTable benchmarks should show improvements.
Given that Gaia DR4 drop is imminent, it would be great to get these VOTable reading improvements in sooner rather than later.
Addresses #18442
Addresses #8191 (BINARY side)
AI Disclosure
I used Claude Opus 5.5 to plan and execute a set of PRs to improve VOTable BINARY parsing performance (speed and memory). After lots of false starts, overly-complex solution and iteration, here we are.
Merge method