Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing of blank values in ECSV subtype data #11720

Merged
merged 4 commits into from
May 11, 2021

Conversation

taldcroft
Copy link
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
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.

Copy link
Contributor

@mhvk mhvk left a comment

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
Copy link
Member

eteq commented Jun 11, 2021

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.

None yet

4 participants