Skip to content

Improve environment setup & fix path errors & exec #12

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

Conversation

malcolmgreaves
Copy link

Added instructions on creating a virtual environment with conda.
Obviates the need to use python3/pip3. Also ensures that the
requirements installation will work: there won't be any conflicts
in the environment.

Fixed the path import errors by adding src/__init__.py, explicitly
importing: from src import .., and by moving the executable Python
scripts to the project's root. Updated README documentation too.

Made the download_model.sh script exectuable and changed it to
use the first bash executable found on the system's $PATH.
Additionally, made the Python scripts not executable (they
should be run with python ... at any rate).

@malcolmgreaves
Copy link
Author

This PR also fixes #7, which overlaps a bit with #8. However, this PR takes a different approach, instead making src a python package and having the executable files live outside the package.

Added instructions on creating a virtual environment with `conda`.
Obviates the need to use `python3/pip3`. Also ensures that the
requirements installation will work: there won't be any conflicts
in the environment.

Fixed the path import errors by adding `src/__init__.py`, explicitly
importing: `from src import ..`, and by moving the executable Python
scripts to the project's root. Updated README documentation too.

Made the `download_model.sh` script exectuable and changed it to
use the first `bash` executable found on the system's `$PATH`.
Additionally, made the Python scripts _not_ executable (they
should be run with `python ...` at any rate).
@malcolmgreaves malcolmgreaves force-pushed the mg_improve_setup_fix_package_errors branch from 99c0c28 to 8d537cc Compare February 14, 2019 21:23
@nkconnor
Copy link

probably more sustainable to add a scripts directory. also src would typically be named something like gpt

@nealmcb
Copy link

nealmcb commented Feb 15, 2019

I find pip a much less intrusive requirement than conda, though I know conda is popular in many circles.
I'd suggest instead sticking with a standard pip aproach, and recommending in the README that people follow common best practices by setting up a venv or conda virtual environment before doing the installs.

@WuTheFWasThat
Copy link
Contributor

yeah I'll just recommend using a virtual environment in the README (personally I like pipenv!)

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.

4 participants