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

BUILD: Update numpy requirement to >=1.20.0,<2.2 #5116

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

SMoraisAnsys
Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys commented Sep 2, 2024

As title says. Changes include:

  • use cdouble instead of the long deprecated complex_ when defining dtype with numpy functions;
  • switch from pytomlpp to toml;
  • update ci workflow to not use caching.

The reason for the switch in toml dependency is related to two facts:

  1. There is a problem when combining Linux + numpy>=2.0+ pytomlpp. Indeed, in Linux, we end up loading the libPyDesktopPlugin.so file (see
    AedtAPI = PyDLL(AedtAPIDll_file)
    ) and it seems to be interfering with how pytomlpp works as we are no longer able to load a TOML file. See
    image
  2. toml does provide read and write capabilities while tomli (a more standard package) does only provide read capabilities.

NOTE: AFAIK mono needs to be reachable (hence the current LIBRARY_PATH extension) for pyedb to be able to load EDBWrapper. Otherwise, instantiating an Edb object will raise an exception. This can be avoided if one installs mono manually but I'm not chaning this as it is probably a standard work around used by our users. At some point, it would be wise to remove dotnetcore/dotnetcore2 and ensure that dotnet + mono are installed if required.

@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added the maintenance Package and maintenance related label Sep 2, 2024
@SMoraisAnsys
Copy link
Collaborator Author

@svandenb-dev This PR won't be able to pass until a new release of pyedb.

Copy link

codecov bot commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.48%. Comparing base (58ef612) to head (736f7df).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5116   +/-   ##
=======================================
  Coverage   83.47%   83.48%           
=======================================
  Files         128      128           
  Lines       57473    57470    -3     
=======================================
  Hits        47977    47977           
+ Misses       9496     9493    -3     

Copy link
Collaborator Author

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

To be added after success of the CI workflow.

.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
.github/workflows/ci_cd.yml Show resolved Hide resolved
_unittest/test_04_SBR.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@SMoraisAnsys SMoraisAnsys marked this pull request as ready for review September 17, 2024 10:07
@SMoraisAnsys
Copy link
Collaborator Author

@Samuelopez-ansys Let me know if you want to remove the changes that are currently added here about using cache in CICD. I had to remove it to ensure that numpy>=2. got used otherwise a local numpy==1.Y.Z was used.

@SMoraisAnsys SMoraisAnsys marked this pull request as draft September 17, 2024 12:27
@SMoraisAnsys
Copy link
Collaborator Author

Seems like the fix is not working when adding back osmnx and merging main. I need to double check if this is related to using numpy<2 due to osmnx.

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

Successfully merging this pull request may close these issues.

2 participants