-
Notifications
You must be signed in to change notification settings - Fork 5
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
Issue in the cell for the glueing #227
Comments
After some investigation, I noticed that the error:
appears when executing the code:
where @dsavchenko maybe you have an idea of this |
I changed the test a bit (see PR), to demonstrate what's going on. The difference is that in the second case, the encoded object is a string (kind of double-encoded in a sense). So it ends up being processed through literal_to_json. And by simple coincidence, there is 'nan' substring in the encoded representation of the binary data. It gets modified, then unsurprisingly the data is corrupted. I don't see a simple and clear solution, the overall encoding-decoding is convoluted, and overall, I don't really understand why it is made like this, with all this pickling etc. |
It seems to affect only products retrieved through the nb2workflow run function, as files downloaded from the front end are correct. Maybe @volodymyrss has a historical overall view. Please notice that also the rmf file in the spectral extraction has an issue related to encoding and decoding. Most probably due to the peculiar nature of the compression used in the matrix or maybe just for the presence of NaN. |
Is it really for this repository? The complexity with encoding in odaapi is old, but the complexity is already with astropy fits serialization, there were astropy bugs with variable size fits tables. I think as long as there are tests, refactoring is safe. |
As noticed by @ferrigno , there is probably an issue with the
decode
function of theImageDataProduct
and theNumpyDataProduct
classhttps://gitlab.renkulab.io/astronomy/mmoda/jemx-expert/-/blob/main/test_products.ipynb?ref_type=heads
The text was updated successfully, but these errors were encountered: