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

docs: add intruction to create virtualenv #1553

Merged
merged 2 commits into from
Jul 10, 2020
Merged

docs: add intruction to create virtualenv #1553

merged 2 commits into from
Jul 10, 2020

Conversation

dmesquita
Copy link
Contributor

@dmesquita dmesquita commented Jul 10, 2020

Hello!
[edited]
This pull request changes the virtual virtual environment link from python tutorials to python docs.
The tutorials page has some text before getting to the command itself, the docs page is more straightforward so the user can paste the code get back to DVC guides :)

@@ -38,6 +38,8 @@ Now let's install the requirements:
> first.

```dvc
$ python3 -m venv .env
Copy link
Member

Choose a reason for hiding this comment

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

this is great, didn't know that it's part of the standard library ... have been using virtualenv -p python3 .env so far

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha yeah it's awesome! venv is recommended since Python 3.6: 28.3. venv — Creation of virtual environments

Comment on lines 38 to 44
> first.

```dvc
$ python3 -m venv .env
$ source .env/bin/activate
$ pip install -r src/requirements.txt
```
Copy link
Contributor

Choose a reason for hiding this comment

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

If this stays, it should be part of the md block quote above, please.

But should it stay? I'm not sure. We purposefully don't include this in our docs any more (we try to avoid having to maintain docs/examples of non-DVC tools). We assume people will know some basics. THAT SAID, maybe in this case it's a good idea to keep it, as this is the most introductory of our docs.

Copy link
Contributor

@jorgeorpinel jorgeorpinel Jul 10, 2020

Choose a reason for hiding this comment

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

Also,

source .env/bin/activate

On Windows it's

.env/Scripts/activate

I know 😠 but that's another possible reason to avoid this.

Copy link
Contributor Author

@dmesquita dmesquita Jul 10, 2020

Choose a reason for hiding this comment

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

Hmm, got it. One cool thing spaCy has is this quick section to generate the command. But it makes more sense there because it's the first page of the Get Started guide. Here the page in question is the "third one" of the guide, I don't know if the pages should follow a sequence or not.

About assuming people will know some basics, maybe it would be good to add here a little more about what you guys assume about the audience of the docs?

Copy link
Member

Choose a reason for hiding this comment

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

@dmesquita I've seen a few projects do a widget likes this for the installation page ... we can do something similar for the dvc.org/doc/install - that would be great. Please, feel free to create a ticket.

@shcheklein
Copy link
Member

thanks @dmesquita !

@shcheklein shcheklein merged commit e149d68 into iterative:master Jul 10, 2020
> [virtual environment](https://packaging.python.org/tutorials/installing-packages/#creating-virtual-environments)
> [virtual environment](https://python.readthedocs.io/en/stable/library/venv.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! We should probably change this link in other places as well. I think it's in a few places.

Copy link
Contributor

Choose a reason for hiding this comment

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

Moved to #1556

@shcheklein
Copy link
Member

One downside of using built in venv is that pip is outdated, wheel is not includes which makes it very inefficient to use to install DVC to my mind. Additional command is needed:

env/bin/python -m pip install --upgrade pip setuptools wheel

more on this here - https://chriswarrick.com/blog/2018/09/04/python-virtual-environments/ .

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jul 24, 2020

Are you saying the change to use the python -m venv .env command was not thought through @shcheklein ? Should we roll back to virtualenv .env (and perhaps the link as well)? See https://github.com/iterative/dvc.org/pull/1556/files

@shcheklein
Copy link
Member

@jorgeorpinel @dmesquita let you decide on this :) (ping the team as well - what do they think).

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.

3 participants