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.

Sign up for GitHub

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

Jump to bottom

"integral field without a specified null value" in BINARY2 VOTable #16090

Open
msdemlei opened this issue Feb 22, 2024 · 3 comments · May be fixed by #16091
Open

"integral field without a specified null value" in BINARY2 VOTable #16090

msdemlei opened this issue Feb 22, 2024 · 3 comments · May be fixed by #16091
Labels
Bug io.votable

Comments

@msdemlei
Copy link
Contributor

Description

When writing VOTable, astropy refuses to write NULL integers with

astropy.io.votable.exceptions.W31: ?:?:?: W31: NaN given in an integral field without a specified null value

even when serialising to BINARY2, which has been created specifically to avoid trouble of this sort.

Expected behavior

A VOTable should be written with the appropriate mask set.

How to Reproduce

Here's a reproducer; only the last four lines actually contain
interesting code, the rest is scaffolding:

import sys
from astropy.io.votable import tree as V
from astropy.io import votable

vot = V.VOTableFile(config={'version_1_2_or_later': True})
resource = V.Resource()
vot.resources.append(resource)
table = V.Table(vot)
resource.tables.append(table)

table.fields.append(V.Field(vot, name="i", datatype="int"))
table.create_arrays(1)
table.array.mask[0] = (True,)
votable.writeto(vot, sys.stdout, "binary2")

The thing fails in the integer's binoutput:

if mask:
     if self.null is None:
         vo_raise(W31)

Somehow, one would need to introduce a switch here so the inner condition only fires when not using BINARY2. I was hoping someone here knows a straightforward way to do that -- is there?

Versions

Linux-6.6.8-g47cb3fd3419e-x86_64-with-glibc2.36
Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]
astropy 5.2.1
Numpy 1.24.2
pyerfa 2.0.0.1
Scipy 1.10.1
Matplotlib 3.6.3
@msdemlei msdemlei added the Bug label Feb 22, 2024
Deen-dot added a commit to Deen-dot/astropy that referenced this issue Feb 22, 2024
@Deen-dot
Fix Binary2 serialization for VOTable
3e652ae 
(Addressing astropy#16090) - Something possibly overlooked in the VOTable 1.3 support implementation

Error in question that was fixed: astropy.io.votable.exceptions.W31: ?:?:?: W31: NaN given in an integral field without a specified null value

In the `_write_binary` method, it seems we are ignoring the fact that the mask for binary2 serialization was already handled before unnecessarily passing the `array_mask` (making the handling useless). This causes an error that would make more sense to be shown in the case of a Binary serialization and not a Binary2 because the bit array indicating the mask is already written to the data stream.
@Deen-dot
Copy link
Contributor

image

Managed to get it to work, I think I found the problem here.

It looks like in _write_binary, we are ignoring the fact that the mask for binary2 serialization was already handled before unnecessarily passing the array_mask (making the handling useless). I added a simple if-condition as a fix, and I can submit a pull request right now.

@Deen-dot Deen-dot linked a pull request Feb 22, 2024 that will close this issue
Draft
1 task
@neutrinoceros
Copy link
Contributor

thanks for reporting this !

Somehow, one would need to introduce a switch here so the inner condition only fires when not using BINARY2. I was hoping someone here knows a straightforward way to do that -- is there?

I'm not an expert in VOTable but I had a look and I don't think that's going to be straightforward.

even when serialising to BINARY2, which has been created specifically to avoid trouble of this sort.

I thought about it in a vacuum, and I think a solution would be to subclass io.votable.converters.Int with something like:

class IntBin2(Int):
    null = ... # insert appropriate value

and use this with tabledata_format="binary2". However, I don't know enough about VOTable that I'm confident this is the right way. Do you have an exact citation on this ? Meanwhile I can search through https://www.ivoa.net/documents/VOTable/20191021/REC-VOTable-1.4-20191021.pdf

@neutrinoceros
Copy link
Contributor

neutrinoceros commented Feb 22, 2024 •
edited by pllim

oops, got scoped by @Deen-dot 😅

EDIT: xref #16091

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug io.votable
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BUG: Fix Binary2 serialization for VOTable Deen-dot/astropy
4 participants
@pllim @msdemlei @neutrinoceros @Deen-dot