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

Python3 documentation/scripts update #40

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

csev1755
Copy link

Changed startup scripts and instructions to python3. Tested on a raspberry pi.

Copy link
Collaborator

@Nocturnal42 Nocturnal42 left a comment

Choose a reason for hiding this comment

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

The changes to call 'python3' instead of 'python' aren't really necessary, since 'python' should be linked to the appropriate python3 binary, but it works either way.

The change to requirements.txt needs to be reverted (or the line removed entirely) and if you add the change I requested to the readme, this should be good for merging.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is unnecessary, and actually breaks the installation with python3. The logging module is included as part of the standard python3 library and so doesn't need to be installed, which is what this flagged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is all mostly fine. Though I really don't like being forced into a virtual environment, there is no good way around it.

Needs an extra bit to add the venv to the path, which will simplify troubleshooting and manual running on the controller, since it will bypass the need to source the venv. Something like this added to the bottom of .bashrc

export PATH="/home/user_name/remotv/.venv/bin:$PATH"

Should save time and trouble since a lot of the users don't know anything about python or virtual environments.

@csev1755
Copy link
Author

@Nocturnal42 Thank you for the review! I made the requested changes. Please let me know if you see anything else that needs to be reconciled.

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