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: formatted info json #38

Merged
merged 1 commit into from
Jul 2, 2024
Merged

fix: formatted info json #38

merged 1 commit into from
Jul 2, 2024

Conversation

xgui3783
Copy link
Collaborator

@xgui3783 xgui3783 commented Jul 1, 2024

I don't know when it started, but at a certain point, nibabel.affines.voxel_sizes(affine) returns list of np types. This makes json.loads(formatted_info) fail, e.g.:

$ pytest unit_tests/test_volume_reader.py

# truncated for brevity

self = <json.decoder.JSONDecoder object at 0x7fdfa94d5f00>
s = '{\n    "type": "image",\n    "num_channels": 3,\n    "data_type": "uint8",\n    "scales": [\n        {\n            "...1000000.0), np.float64(1000000.0), np.float64(1000000.0)],\n            "voxel_offset": [0, 0, 0]\n        }\n    ]\n}'
idx = 0

    def raw_decode(self, s, idx=0):
        """Decode a JSON document from ``s`` (a ``str`` beginning with
        a JSON document) and return a 2-tuple of the Python
        representation and the index in ``s`` where the document ended.
    
        This can be used to decode a JSON document from a string that may
        have extraneous data at the end.
    
        """
        try:
            obj, end = self.scan_once(s, idx)
        except StopIteration as err:
>           raise JSONDecodeError("Expecting value", s, err.value) from None
E           json.decoder.JSONDecodeError: Expecting value: line 9 column 28 (char 187)

/usr/lib/python3.10/json/decoder.py:355: JSONDecodeError

It should be noted that on nibabel master branch, a test failed seemingly for the same bug: https://github.com/nipy/nibabel/actions/runs/9705672252/job/26788138447#step:7:200

I propose this PR, which should fix the problem in the interim, and would still be backwards compatible when/if nibabel team fixes this issue.

(also added underscore for number readability (https://peps.python.org/pep-0515/ , supported from py3.6 onwards)

ping @ylep

@xgui3783
Copy link
Collaborator Author

xgui3783 commented Jul 1, 2024

hmm, I will double check the failed tests.

@ylep
Copy link
Collaborator

ylep commented Jul 1, 2024

Thanks @xgui3783 ! The failed tests are unrelated, they are due to major updates in NumPy 2.0. I am working on fixing them!
Also, I have added a scheduled weekly run of the tests so that we can detect this kind of breakage earlier

@ylep ylep merged commit afcda34 into master Jul 2, 2024
6 of 12 checks passed
@ylep ylep deleted the bugfix_volume_info branch July 2, 2024 11:40
@ylep
Copy link
Collaborator

ylep commented Jul 2, 2024

Merged through #40.
Thanks again @xgui3783! I will try to make a release, because the PyPI version currently generates invalid data with NumPy 2.0, which could be really problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants