Skip to content

Conversation

@fkroeber
Copy link
Collaborator

Description

Numpy version 2 requires np.nan instead of np.NaN.

Type of change

Select one or more relevant options:

  • Bug fix (change which fixes a bug reported in a specific issue)
  • New feature (change which adds a feature requested in a specific issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (improved documentation for existing functionality in the package)
  • Repository update (change related to non-package files or structures in the GitHub repository)

Checklist:

  • I have read and understood the contributor guidelines of this project
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I ran all demo notebooks locally and there where no errors

@augustinh22
Copy link
Collaborator

@fkroeber If we make this change, I think we have to change the dependency requirements of semantique to be at least numpy v2, which will require a bit more checking of functionality for potential breaking changes. It seems a few packages (z.B. geopandas) went through a large version change in June last year, so this is likely a larger project to tackle.

https://github.com/numpy/numpy/releases/tag/v2.0.0

This will be useful to go though to make sure nothing else is impacted:

https://numpy.org/doc/stable/numpy_2_0_migration_guide.html

@fkroeber
Copy link
Collaborator Author

@augustinh22 you are of course right: A comprehensive review of the implementation is necessary, to ensure complete numpy v2 compatibility. Note, however, that this particular commit could be integrated without any changes in the dependency requirements (since np.nan works with numpy v1 as well). So we can either integrate this PR and open a new issue on the general numpy v2 compatibility, or leave this PR open and rename it to encompass all necessary numpy v2 changes.

@whisperingpixel
Copy link
Collaborator

In general, it touches a very similar topic as this PR #59 (already merged) regarding the dependencies in semantique.

Since you tested it and confirm the downwards-compatibility of the change, I think we could merge it. Everyone who wants to be on the safe side can still use numpy < V2.

It would be necessary, however, to keep in mind to make a general update regarding the dependencies and make sure that everything is up-to-date. On the long run, we should not stick too long with old versions.

@mattyfromtheblock
Copy link
Contributor

From what I've gathered, the actual change here is really just a matter of a deprecated "alias" (i.e. "np.nan" is still valid in numpy < 2.0 and is used throughout semantique already). In any case, I came across this issue myself while doing some testing related to #61 (I should have checked the PRs beforehand, apologies!).

As for any breaking changes possibly introduced with numpy >= 2.0: I did actually run a couple tests using numpy 2.2.2 which all completed successfully. While this should not be counted as sufficient testing, at least we have an indication that things won't break right away (whether this is promising or worrying sorta depends on your personal inclination towards optimism/pessimism I suppose).

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.

4 participants