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

Use maturin v1.0.0-beta.5 with PEP 704 (.venv) support #7782

Closed
wants to merge 4 commits into from

Conversation

konstin
Copy link

@konstin konstin commented Mar 25, 2023

I recently add support to maturin to automatically use .venv from PEP 704 and i'm now trying to integrate this into real world projects.

Depending on your preference you might want to wait for the stable v1.0 maturin release before merging this.

@stinodego
Copy link
Contributor

stinodego commented Mar 26, 2023

venv is a Make command that relies on the venv folder being present. We would have to rename that to .venv, which I think is fine but I'm not sure is allowed?

Update: Tested that, and it works.

Copy link
Contributor

@stinodego stinodego left a comment

Choose a reason for hiding this comment

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

I'd be in favor of this change - I think .venv is more standardized as a folder than the current venv folder we use. But please do a search for venv through the code base, because there are some other spots where we rely on the venv folder, i.e. .gitignore and the Make command venv which should then be renamed to .venv.

Also, I think this change might also come in helpful in the CI?

And personally, I'd like to wait for the stable release, otherwise we'd have to update again when that comes out.

EDIT: Let me make that .venv rename change myself, then we can keep this PR purely for the maturin change.

@konstin
Copy link
Author

konstin commented Mar 26, 2023

rebase to avoid merge conflicts once #7790 is merged

@stinodego
Copy link
Contributor

I'll close this for now, and open a new PR whenever maturin 1.0.0 comes out.

@stinodego stinodego closed this Apr 10, 2023
konstin added a commit to konstin/polars that referenced this pull request May 8, 2023
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.

2 participants