-
Notifications
You must be signed in to change notification settings - Fork 52
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
Byteswap in all cases where bpp>1 ... #310
Labels
Comments
I do not understand why the bpp must be checked at all here. How to apply byte swapping for |
boesecke
added a commit
to boesecke/fabio
that referenced
this issue
Jun 12, 2019
- global variable ROOT removed edfimage.py: - get_data_rank etc. defined as static function of class EdfFrame - function description strings updated with comment strings and latter removed - _parseheader (parseheader) renamed to _create_header. It better describes it. - _read_header_block returns a namedtuple - There are some comments left in the code. Most of them should be removed in the final version. Concerning byteswapping, there was already an issue opened for updating it (Byteswap in all cases where bpp>1 ... silx-kit#310). When this is solved the internal comment should be removed.
I propose the following simplification:
Replacing:
|
Rather the simplifying the code, can you just remove it altogether?
See: https://commandcenter.blogspot.com/2012/04/byte-order-fallacy.html
He suggests one should try to remove "swap" lines of code. So rather
than fixing "self._data_swap_needed" can numpy do this when reading the
bytes by specifying their flavour? Just set self.bytecode to include the
source endianness as a little "<" or big ">" as first character. e.g:
>> t4 = numpy.array( (1,0,0,0), numpy.uint8 )
>> numpy.frombuffer( t4, dtype='>u4')
array([16777216], dtype=uint32)
>> numpy.frombuffer( t4, dtype='<u4')
array([1], dtype=uint32)
There is no need for a special case on the 1 bpp:
>> numpy.frombuffer( t4, dtype='<u1')
array([1, 0, 0, 0], dtype=uint8)
>> numpy.frombuffer( t4, dtype='>u1')
array([1, 0, 0, 0], dtype=uint8)
|
On Wed, 12 Jun 2019 09:16:23 -0700 Jon Wright ***@***.***> wrote:
Rather the simplifying the code, can you just remove it altogether?
See: https://commandcenter.blogspot.com/2012/04/byte-order-fallacy.html
He suggests one should try to remove "swap" lines of code. So rather
than fixing "self._data_swap_needed" can numpy do this when reading the
bytes by specifying their flavour? Just set self.bytecode to include the
source endianness as a little "<" or big ">" as first character. e.g:
Indeed this would be simpler...
I suggest to open a separated issue/PR as this pattern affects (almost)
all file-formats and upgrading them in a consistent way.
There could be a static function in fabioimage providing the "sexed_dtype"
```
@statimethod
def get_sexed_dtype(dtype, endianness):
if endianness=="little":
stype = "<"
elif endianness=="big":
stype = ">"
else:
stype = ""
return stype+numpy.dtype(dtype).char
```
which could be used in the 20 classes where this pattern is found ?
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Spotted in edfimage at least, by Peter Boesecke. Probably affects only "complex128" with swapped endianness.
The text was updated successfully, but these errors were encountered: