Skip to content

Fix parsing of blank values in ECSV subtype data - #11720

Merged
taldcroft merged 4 commits into
astropy:mainfrom
taldcroft:ascii-ecsv-blank-value-subtypes
May 11, 2021
Merged

taldcroft merged 4 commits into
astropy:mainfrom
taldcroft:ascii-ecsv-blank-value-subtypes

Conversation

@taldcroft

Copy link
Copy Markdown
Member

Description

This pull request is another follow-on to #11569 and fixes the implementation to correctly parse fields that have a blank (empty string) value in subtype columns (object, fixed array, variable array). The APE-6 update will now specify that blank values are allowed in subtype columns as an indicator of missing data.

This is obviously late. If need be this fix could be pushed to the first bug-fix release for 4.3, but getting it in for the 4.3 release itself is preferred.

@mhvk @mbtaylor

@mbtaylor

Copy link
Copy Markdown
Contributor

Thanks @taldcroft , I think this change in the specification and implementation will make the ECSV subtypes a more useful thing to have. Apologies my earlier dithering on this contributed to lateness of the update; I agree it would be nice if possible for it to appear with the initial ECSV implementation rather than a later bugfix.

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Happy to approve but perhaps good to rebase to ensure that all the tests pass (current failure is resolved in main, but it blocks other tests from running)

@taldcroft
taldcroft force-pushed the ascii-ecsv-blank-value-subtypes branch from 7aebddb to 66cbe85 Compare May 11, 2021 12:32
@taldcroft
taldcroft force-pushed the ascii-ecsv-blank-value-subtypes branch from 66cbe85 to 1ba9c59 Compare May 11, 2021 13:04
@taldcroft
taldcroft merged commit a1c35fc into astropy:main May 11, 2021
@taldcroft
taldcroft deleted the ascii-ecsv-blank-value-subtypes branch May 11, 2021 15:44
eteq pushed a commit that referenced this pull request Jun 11, 2021
Fix parsing of blank values in ECSV subtype data
@eteq

eteq commented Jun 11, 2021

Copy link
Copy Markdown
Member

backported to v4.3.x in 54f58bc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants