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

Loosen version requirement for maturin #1005

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Conversation

gyscos
Copy link
Contributor

@gyscos gyscos commented Dec 9, 2022

Description

This loosen the version requirement for maturin when building the python package.

Related Issue(s)

Documentation

Using an exact version can be excessively restrictive, for example preventing the use of the latest (but still compatible) version of maturin. Especially when packaging for rolling-releases distributions (like Archlinux), being able to build with the latest version would be beneficial.

@gyscos
Copy link
Contributor Author

gyscos commented Dec 9, 2022

Ah I may need to update python/Makefile...

@gyscos
Copy link
Contributor Author

gyscos commented Dec 9, 2022

Makefile updated to properly parse the maturin version.

(Note that python-build may simplify the build process, using virtualenv and maturin as expected, and removing the need for manual config file parsing.)

@wjones127
Copy link
Collaborator

(Note that the entire build step could also be done as simply as python -m build, and it will use virtualenv and maturin as expected. Was there a reason for the more complex Makefile solution?)

That's an interesting point. I'll want to try that out and see how it works in the development workflow, but potentially could be a useful way to go.

Copy link
Collaborator

@fvaleye fvaleye left a comment

Choose a reason for hiding this comment

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

Good catch, thanks @gyscos 👍

@wjones127 wjones127 merged commit e0f953b into delta-io:main Dec 9, 2022
@gyscos
Copy link
Contributor Author

gyscos commented Dec 9, 2022

It looks like one test timeouted, not sure if it's related to this PR.
EDIT: once merged in main, the test passed, so 🤷

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.

Python package: Loosen version requirements for maturin
3 participants