Skip to content

Conversation

LecrisUT
Copy link
Collaborator

Fixes #212

@LecrisUT
Copy link
Collaborator Author

@atztogo Can you confirm this works? I will upstream this fix to scikit-build later if it does

@LecrisUT LecrisUT mentioned this pull request Jan 12, 2023
@atztogo
Copy link
Collaborator

atztogo commented Jan 13, 2023

@LecrisUT, thanks for your quick work. Almost. The following change at least made it work.

% git diff setup.py
diff --git a/python/setup.py b/python/setup.py
index f5d40e1..42a79eb 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -5,9 +5,9 @@ from skbuild import setup
 # TODO: Temporary fix for #212
 #  Remove and update dependency when scikit-build/scikit-build#717 is resolved
 import platform
-if platform.version() == 'Darwin':
+if platform.system() == 'Darwin':
     mac_version, _, mac_arch = platform.mac_ver()
-    mac_version = f"{mac_version}-{mac_arch}"
+    mac_version = f"{mac_version}"
 else:
     mac_version = None

@@ -75,7 +75,7 @@ setup(
     tests_require=[
         "pyyaml",
     ],
-    cmake_args=['-DWITH_Python=ON', '-DWITH_TESTS=OFF', '-DUSE_OMP=OFF', '-DBUNDLE_Python_SharedLib=ON'],
+    cmake_args=cmake_args,
     cmake_source_dir="..",
     extras_require=extras_require,
 )

Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
@LecrisUT
Copy link
Collaborator Author

Oh god. I've derped that commit bad. Sorry about that. I am surprised why we don't need to set up CMAKE_OSX_ARCHITECTURES as well.

@codecov-commenter
Copy link

Codecov Report

Base: 64.34% // Head: 64.00% // Decreases project coverage by -0.33% ⚠️

Coverage data is based on head (b176785) compared to base (b8d068b).
Patch has no changes to coverable lines.

❗ Current head b176785 differs from pull request most recent head 15fd0b3. Consider uploading reports for the commit 15fd0b3 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #214      +/-   ##
===========================================
- Coverage    64.34%   64.00%   -0.34%     
===========================================
  Files           18       18              
  Lines         1321     1328       +7     
===========================================
  Hits           850      850              
- Misses         471      478       +7     
Impacted Files Coverage Δ
setup.py 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@atztogo atztogo merged commit fb55b8f into spglib:develop Jan 13, 2023
@atztogo
Copy link
Collaborator

atztogo commented Jan 13, 2023

Merged. Thanks @LecrisUT. Your PR worked on my macOS.

@LecrisUT LecrisUT deleted the fix/212 branch January 20, 2023 09:36
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.

pip install doesn't work for macosx (arm64)
3 participants