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

fits.PrimaryHDU.fromstring() errors when passing bytearray #15132

Closed
jaokeeffe opened this issue Aug 5, 2023 · 10 comments · Fixed by #15155
Closed

fits.PrimaryHDU.fromstring() errors when passing bytearray #15132

jaokeeffe opened this issue Aug 5, 2023 · 10 comments · Fixed by #15155

Comments

@jaokeeffe
Copy link
Contributor

Description

I'm trying to create an HDU from a bytearray that includes the header and data. According to the documentation I can pass in a bytearray. However when I try I get an error saying: TypeError: sequence item 0: expected str instance, bytearray found

It seems to be interpreting the bytearray as a bytearray but doesn't like it.

image

Expected behavior

HDU created with the header and image data.

How to Reproduce

  1. FITS file loaded into a bytearray
  2. Try and create a new PrimaryHDU with the astropy.io.fits.PrimaryHDU.fromstring() method passing in the bytearray and the ignore_missing_end=True option.

Versions

Linux-5.15.0-1041-azure-x86_64-with-glibc2.35
Python 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0]
astropy 5.3.1
Numpy 1.21.5
pyerfa 2.0.0.3
Scipy 1.9.1
Matplotlib 3.5.2

@jaokeeffe jaokeeffe added the Bug label Aug 5, 2023
@github-actions
Copy link

github-actions bot commented Aug 5, 2023

Welcome to Astropy 👋 and thank you for your first issue!

A project member will respond to you as soon as possible; in the meantime, please double-check the guidelines for submitting issues and make sure you've provided the requested details.

GitHub issues in the Astropy repository are used to track bug reports and feature requests; If your issue poses a question about how to use Astropy, please instead raise your question in the Astropy Discourse user forum and close this issue.

If you feel that this issue has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail feedback@astropy.org.

@pllim pllim added the io.fits label Aug 6, 2023
@pllim
Copy link
Member

pllim commented Aug 6, 2023

@jaokeeffe
Copy link
Contributor Author

Thanks for the quick reply!

I have tried this, and as long as I convert it to a sting with UTF-8 encoding, then I am able to interpret the header. If I use the PrimaryHDU class with fromstring() method then it doesn't. The way I am reading the documentation, if I use the PrimaryHDU class, then I don't need to specify the encoding. The FITS file is read into a variable of type bytearray which includes both the header and the image data (a flat frame in this case). I was hoping to re-create a PrimaryHDU from this datatype without having to separate the header from the data. Sorry if I am missing something simple and obvious!

image

@dhomeier
Copy link
Contributor

dhomeier commented Aug 7, 2023

Have you tried this with only the header part (i.e. using header_bytes but without decoding to string)?
I don't think fromstring would (or should) work on any byte sequence including the data part, as this will likely include lots of invalid "characters".
That approach will probably still not be very robust, as the header section can in principle span any multiple of 2880 bytes.

@jaokeeffe
Copy link
Contributor Author

Thanks Derek for the reply.

Seems that the documentation is a little misleading. It indicates that you can pass a bytearray object to the fromstring() method, but in fact you need to pass a bytes object instead (like the Header class documentation says). When I cast the bytesarray object to bytes it works fine and I can see the header and data.

Here is the docs page I am referring to: https://docs.astropy.org/en/stable/io/fits/api/hdus.html#astropy.io.fits.PrimaryHDU.fromstring

image

@pllim
Copy link
Member

pllim commented Aug 10, 2023

@jaokeeffe , thanks for investigating! Are you interested to open a PR to fix the doc?

@dhomeier
Copy link
Contributor

Yes, this whole code has been in since merging PyFITS in #2575 at least, and it's not clear if this was ever supposed to function with bytearray (it does work up to creating the data section), so we should probably consider it to mean bytes really (which is also what open(fitsfile, 'rb') returns and what is tested).

@jaokeeffe
Copy link
Contributor Author

jaokeeffe commented Aug 11, 2023

PR #15155 Opened for docs fix. Hopefully I did the right thing and didn't mess things up!

@pllim
Copy link
Member

pllim commented Aug 11, 2023

Thanks!

@saimn
Copy link
Contributor

saimn commented Aug 29, 2023

bytearray was probably supported a while ago, but since there are no tests with it some changes may have broken it. Probably easy to fix if someone wants to, but until then I agree updating the docs is the easiest way to go.

saimn added a commit that referenced this issue Aug 29, 2023
…tes not bytearray for data parameter (#15155)

* Replaced bytearray with bytes in documentation for fromstring() method of PrimaryHDU class to reflect it's correct usage. [skip ci]

* Add changelog entry

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Delete docs/changes/coordinates/15155.other.rst

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Simon Conseil <contact@saimon.org>
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this issue Aug 29, 2023
…PrimaryHDU.fromstring() accepts bytes not bytearray for data parameter
pllim added a commit that referenced this issue Aug 30, 2023
…155-on-v5.3.x

Backport PR #15155 on branch v5.3.x (Documentation fix for issue #15132 PrimaryHDU.fromstring() accepts bytes not bytearray for data parameter )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants