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.

Sign up for GitHub

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

Jump to bottom

DOCS-1111: Add virtualenv instructions for Python SDK #1830

Merged
merged 14 commits into from
Sep 15, 2023
Merged

DOCS-1111: Add virtualenv instructions for Python SDK #1830

merged 14 commits into from
Sep 15, 2023

Conversation

sguequierre
Copy link
Collaborator

@sguequierre sguequierre commented Sep 14, 2023 •
edited
Loading

Big ask for HackTheNorth from PennApps crew

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Sep 14, 2023
@viambot
Copy link
Member

viambot commented Sep 14, 2023 •
edited
Loading

Overall readability score: 54.7 (🟢 +0.04)

File Readability
python-venv.md 68.49 (-)
View detailed metrics

🟢 - Shows an increase in readability
🔴 - Shows a decrease in readability

File Readability FRE GF ARI CLI DCRS
python-venv.md 68.49 49.31 8.13 10.1 11.42 6.73
  - - - - - -

Averages:

  Readability FRE GF ARI CLI DCRS
Average 54.7 46.07 10.87 13.33 11.82 7.76
  🟢 +0.04 🟢 +0.01 🟢 +0.01 🟢 +0.01 🟢 +0 🟢 +0
View metric targets
Metric Range Ideal score
Flesch Reading Ease 100 (very easy read) to 0 (extremely difficult read) 60
Gunning Fog 6 (very easy read) to 17 (extremely difficult read) 8 or less
Auto. Read. Index 6 (very easy read) to 14 (extremely difficult read) 8 or less
Coleman Liau Index 6 (very easy read) to 17 (extremely difficult read) 8 or less
Dale-Chall Readability 4.9 (very easy read) to 9.9 (extremely difficult read) 6.9 or less

@sguequierre sguequierre requested a review from mcvella September 14, 2023 17:54
@sguequierre sguequierre marked this pull request as ready for review September 14, 2023 18:18
mcvella
mcvella approved these changes Sep 14, 2023
View reviewed changes
Copy link
Contributor

@mcvella mcvella left a comment

Choose a reason for hiding this comment

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

small suggested changes, else looks good

docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
npentrel
npentrel requested changes Sep 14, 2023
View reviewed changes
docs/program/python-venv.md Outdated
Comment on lines 30 to 31
> **INFO**
> Creating a virtual environment (`venv`) is important as it isolates this python environment from any other you might already have.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We currently don't have this type of styling in our style guide, while we can add it we'd need to discuss that as a team first.

Let's stick with an alert until then

Copy link
Collaborator

Choose a reason for hiding this comment

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

However, this also duplicates the info above. in the first paragraph on the page. I think we should not duplicate that.

docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated
source viam-env/bin/activate
```

Now, you should see `(viam-env)` prepend the commands in your terminal window.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove conditional

docs/program/python-venv.md Outdated
```

Now, you should see `(viam-env)` prepend the commands in your terminal window.
This shows that the python packages being used are from this particular environment.
Copy link
Collaborator

Choose a reason for hiding this comment

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

try remove passive voice

docs/program/python-venv.md Outdated

## Setup your IDE

You'll now want to point your IDE to use the python interpreter of your new environment, rather than the default interpreter, likely the global python interpreter.
Copy link
Collaborator

Choose a reason for hiding this comment

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

avoid future tense - instead explain why they want that "If you awnt to ... then"

docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Show resolved Hide resolved
docs/program/python-venv.md Outdated
Comment on lines 74 to 76
## Start building

You are now ready to start using Viam's Python SDK!
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should instead point the user to the next step. Probably back to the /program page where they can get the code sample

@npentrel
Copy link
Collaborator

Good to merge if you need this quickly but let's then make a ticket to address this feedback separately.

sguequierre
sguequierre commented Sep 15, 2023
View reviewed changes
docs/program/python-venv.md Outdated
---
title: "Prepare your Python Virtual Environment"
linkTitle: "Prepare your Python Virtualenv for Viam"
weight: 50
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
weight: 50
weight: 10

sguequierre
sguequierre commented Sep 15, 2023
View reviewed changes
docs/program/python-venv.md Outdated
source viam-env/bin/activate
```

Now, you should see `(viam-env)` prepend the commands in your terminal window.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
Now, you should see `(viam-env)` prepend the commands in your terminal window.
Now, watch `(viam-env)` prepend the commands in your terminal window.

npentrel
npentrel reviewed Sep 15, 2023
View reviewed changes
Copy link
Collaborator

@npentrel npentrel left a comment

Choose a reason for hiding this comment

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

A few more small things

docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
docs/program/python-venv.md Outdated Show resolved Hide resolved
@viambot
Copy link
Member

viambot commented Sep 15, 2023

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/a8992fcde2bb3a72c7be0506eedf2bc6cc572e2d/public

@sguequierre sguequierre merged commit 679b9af into viamrobotics:main Sep 15, 2023
6 checks passed
@sguequierre sguequierre deleted the DOCS-1111/add-python-sdk-venv-documentation branch September 15, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npentrel npentrel npentrel approved these changes

@mcvella mcvella mcvella approved these changes

Assignees
No one assigned
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sguequierre @viambot @npentrel @mcvella