Skip to content

unpin numpy to enable working with numpy version 2 as well#689

Merged
lukaspie merged 7 commits intomasterfrom
support-npv2
Sep 30, 2025
Merged

unpin numpy to enable working with numpy version 2 as well#689
lukaspie merged 7 commits intomasterfrom
support-npv2

Conversation

@lukaspie
Copy link
Collaborator

@lukaspie lukaspie commented Aug 6, 2025

Connected to the discussion of issue #687 in preparation for amongst other reason NOMAD releases beyond 1.3.17

@lukaspie lukaspie mentioned this pull request Aug 6, 2025
3 tasks
@mkuehbach mkuehbach changed the title unpin numpy unpin numpy to enable working with numpy version 2 also Aug 6, 2025
@lukaspie lukaspie linked an issue Aug 7, 2025 that may be closed by this pull request
3 tasks
@lukaspie lukaspie force-pushed the support-npv2 branch 2 times, most recently from f553399 to e5f9287 Compare August 12, 2025 09:03
@lukaspie lukaspie changed the title unpin numpy to enable working with numpy version 2 also unpin numpy to enable working with numpy version 2 as well Aug 12, 2025
@lukaspie lukaspie marked this pull request as ready for review August 13, 2025 08:43

elif type_id == "str":
value = string_(value).astype(str)
value = value.decode("utf-8") if isinstance(value, bytes) else value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wish to support arbitrary string payload then we need to think about what happens when decode() raises a UnicodeError

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its okay to address this via #705

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that the whole hdfdict.py is just a copy of https://github.com/SiggiGue/hdfdict/blob/master/hdfdict/hdfdict.py (which we had to copy because we wanted to have a stable version and there was no release management on the existing tool at the time). I only made changes here that are related to upgrading to numpy>2, so any decoding problems we had before will be the same. We can discuss this elsewhere later.

name=key, data=yaml.safe_dump(value).encode("utf-8")
)
ds.attrs.create(name=TYPE, data=b"yaml")
# if this fails again, restructure your data!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"restructure your data" should be emitted as a message if it fails again

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, I did not change the code. Could be made more robust in the future, but not here.

Copy link
Collaborator

@mkuehbach mkuehbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments only

@lukaspie
Copy link
Collaborator Author

@mkuehbach thanks for the comments, but as outlined above, I don't plan to address any shortcomings of the copied hdfdict module here, just make cosmetic updates to be able to use np v2. I'll merge here.

@lukaspie lukaspie merged commit 4eafc20 into master Sep 30, 2025
16 checks passed
@lukaspie lukaspie deleted the support-npv2 branch September 30, 2025 12:46
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.

NumPy 2

3 participants