davidcaron / pye57

Read and write e57 point clouds from Python
MIT License
70 stars 41 forks source link

fix: macOS ci build wheels env variable update #67

Closed amangupta17 closed 3 months ago

amangupta17 commented 3 months ago

Building macOS wheels was using the wrong CIBW variable. Variable to be used is CIBW_BEFORE_ALL_MACOS instead of CIBW_BEFORE_ALL_LINUX.

https://cibuildwheel.pypa.io/en/stable/options/#before-all

dancergraham commented 3 months ago

great that's fixed it !

dancergraham commented 3 months ago

ahh no with this release now pye57 will not install properly on mac or on my Windows machine - I will revert everything for now

amangupta17 commented 3 months ago

What is the error you see? Also is there a way to build and test the wheels?

I am able to do pip install . and its works correctly. Also built a macOS wheel locally and then installed it using pip which also worked correctly.

amangupta17 commented 3 months ago

Here's the latest install on macOS.

pip install pye57
Collecting pye57
  Obtaining dependency information for pye57 from https://files.pythonhosted.org/packages/6e/92/ade4fee7744784c1b85cb3a9a62b7605cd8833b8e3b5297420180bff3dad/pye57-0.4.6-cp39-cp39-macosx_11_0_arm64.whl.metadata
  Downloading pye57-0.4.6-cp39-cp39-macosx_11_0_arm64.whl.metadata (4.2 kB)
Requirement already satisfied: numpy in /Users/name/workplace/pye57/venv/lib/python3.9/site-packages (from pye57) (2.0.1)
Requirement already satisfied: pyquaternion in /Users/name/workplace/pye57/venv/lib/python3.9/site-packages (from pye57) (0.9.9)
Downloading pye57-0.4.6-cp39-cp39-macosx_11_0_arm64.whl (1.5 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.5/1.5 MB 10.3 MB/s eta 0:00:00
Installing collected packages: pye57
Successfully installed pye57-0.4.6
dancergraham commented 3 months ago

Do you have xerces C installed on your mac ? For me it failed on mac with python 3.10 and no xerces C saying No such file or directory 'usr/local/lib/libxerces-c... On windows I get ValueError: path '/home/runner/work/pye57/pye57/libE57Format/src/BlobNode.cpp' cannot be absolute On windows python -m pip install --upgrade pye57==0.4.5 works correctly

dancergraham commented 3 months ago

cibuildwheel should have a facility for testing wheels but I don't think we're using it right now - it would be good to add it, making sure that it uses a fresh environment without the build dependencies...

amangupta17 commented 3 months ago

Unsure on the windows error. Maybe the libe57 got updated and has changes around that file. Will need to dig deeper. There were no changes to the windows in my commits so not sure where that came from.

For macOs, I don't have xerces installed locally, I will give it a shot on another machine.

For now it will be great if you can suggest ways to test on all OS. I dont want to create VMs.

dancergraham commented 3 months ago

I think adding an environment variable should do it ! https://cibuildwheel.pypa.io/en/stable/options/#test-command