-
Notifications
You must be signed in to change notification settings - Fork 838
Fix build errors #832
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
Fix build errors #832
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR cleans up the build setup by removing deprecated fetch_build_eggs()
calls and updating how NumPy include paths are acquired, then bumps the package version.
- Removed
fetch_build_eggs()
fallbacks for Cython and NumPy insetup.py
- Switched from
np_get_include()
tonp.get_include()
with animport numpy as np
- Updated version in
pyproject.toml
to0.15.5dev
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
setup.py | Dropped fetch_build_eggs logic, imported NumPy, updated include |
pyproject.toml | Bumped version from 0.15.4 to 0.15.5dev |
Comments suppressed due to low confidence (3)
pyproject.toml:1
- Add a
[build-system]
section inpyproject.toml
(e.g.,requires = ["setuptools>=61", "wheel", "cython<=0.29.34", "numpy"]
,build-backend = "setuptools.build_meta"
) so that Cython and NumPy are installed before building extensions.
[project]
pyproject.toml:3
- [nitpick] The version string
0.15.5dev
may not follow PEP 440 strict conventions; consider using0.15.5.dev0
for a proper development release marker.
version = "0.15.5dev"
setup.py:47
- [nitpick] The top-level
include_dirs
insetup()
may be redundant since eachExtension
already specifiesinclude_dirs
; you can remove it to reduce duplication.
include_dirs=[np.get_include()],
… install instruction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
A PR that fixes build errors by updating installation commands in GitHub Action files and the package setup.
- Updates dependency fetching in setup.py to simplify use of Cython and numpy.
- Revises installation commands in docs and CI workflows to use pip install -e . and support newer Python versions.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
setup.py | Adjusts dependency fetching and installation settings for Cython and numpy. |
pyproject.toml | Updates package version to reflect development progress. |
docs/installation.rst | Revises installation instructions, removing deprecated build commands. |
.github/workflows/* | Updates Python version matrices and switches to pip install -e . for consistency. |
Proposed changes
This PR fixes the build errors generated after #831, which removed the runtime dependency of Cython (e.g., here), by using
pip install -e .
instead ofpython setup.py build_ext --inplace
in the GitHub Action files. The installation document has been updated accordingly.Types of changes
What types of changes does your code introduce to CausalML?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
n/a