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

chore(python): Rename venv folder to .venv #7790

Merged
merged 1 commit into from
Mar 26, 2023

Conversation

stinodego
Copy link
Contributor

@stinodego stinodego commented Mar 26, 2023

Triggered by #7782

I actually wanted to make this change a while ago. The naming of the venv folder does not matter much, but I think .venv is a bit nicer than simply venv for a few reasons, explained well in PEP 704:

The name .venv was picked since it:

  • does not conflict with any valid Python import name
  • does not conflict venv module in the standard library
  • has pre-existing usage in the Python community
  • has support for auto-detection in common text editors
  • can be typed without modifier keys on common keyboard layouts

Now that maturin will offer some automatic support when using .venv, I think that's a valid reason for switching.

@github-actions github-actions bot added chore Maintenance work that does not impact the user python Related to Python Polars labels Mar 26, 2023
Copy link

@konstin konstin left a comment

Choose a reason for hiding this comment

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

this is great, thanks!

@alexander-beedie
Copy link
Collaborator

(Doh... for some reason I thought you already committed this 😅 - you'll need a quick rebase).

@stinodego
Copy link
Contributor Author

(Doh... for some reason I thought you already committed this 😅 - you'll need a quick rebase).

That's fine, I've become quite adept at rebasing 😬 and this is about as simple as they come.

@stinodego stinodego merged commit 8763812 into pola-rs:master Mar 26, 2023
@alexander-beedie
Copy link
Collaborator

alexander-beedie commented Mar 27, 2023

One thing here - I wouldn't remove the original venv from the gitignore quite yet, or 18000 files will suddenly spring into being until the existing venv dir is deleted and recreated in favour of .venv 😇

@stinodego
Copy link
Contributor Author

Yea in hindsight that was unnecessary - I'd approve a PR that re-adds it, however, probably best to delete the old folder and re-create the virtual env anyway, right?

@alexander-beedie
Copy link
Collaborator

alexander-beedie commented Mar 28, 2023

Yea in hindsight that was unnecessary - I'd approve a PR that re-adds it, however, probably best to delete the old folder and re-create the virtual env anyway, right?

For sure - but it causes minor havoc until you do so; I just ran a pre-commit and typos went absolutely bananas, spraying megabytes of Jupyter notebook text into my terminal 🤣 (I guess it uses the gitignore to help it determine which directories to look at). I'll add venv back in to the PR I'm about to raise to smooth things over for now 👍

Update: here it is - #7830

LdRoW pushed a commit to LdRoW/polars that referenced this pull request Mar 28, 2023
konstin pushed a commit to konstin/polars that referenced this pull request May 8, 2023
@stinodego stinodego deleted the dot-venv branch May 25, 2023 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance work that does not impact the user python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants