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

added -y option to docker_build.sh. I also fixed a bug on -p option #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

masayang
Copy link

I had added a couple of features to the sample template. I also fixed a bug in docker_build.sh

  • Now -p option works correctly.
  • By providing -y option, you can specify a version of Python. Default is 3.8
  • If pipenv is in the path, requirements.txt will be generated through pip freeze
  • If jupyter is in the path, workflows/*.ipynb will be transformed to *.py files.

@welcome
Copy link

welcome bot commented Jan 31, 2023

Thank you for opening this pull request! 🙌
These tips will help get your PR across the finish line: - Most of the repos have a PR template; if not, fill it out to the best of your knowledge. - Sign off your commits (Reference: DCO Guide).

@@ -6,25 +6,35 @@ set -e
REGISTRY=""

# SET the appname here
PROJECT_NAME="{{ cookiecutter.project_name }}"
PROJECT_NAME="spx_prediction"
Copy link
Contributor

Choose a reason for hiding this comment

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

can we revert this to the cookiecutter variable?

Comment on lines +53 to +67
# convert *.ipynb to *.py
if hash jupyter 2>/dev/null; then
echo "Jupyter found. converting workflows/*.ipython to *.py"
jupyter nbconvert workflows/*.ipynb --to script
else
echo "Jupyter not found"
fi

# generate requirements.txt if pipenv were used
if hash pipenv 2>/dev/null;then
echo "Pipenv found. Generating requirements.txt"
pipenv run pip freeze > requirements.txt
else
echo "Pipenv not found"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

this kind of expands the scope and purpose of this docker_build.sh script to be opinionated about jupyter and pipenv as developer tools. I think this would make sense perhaps as part of steps a user manually runs in the README.md instructions.

Thoughts @eapolinario ?

@cosmicBboy
Copy link
Contributor

also, @masayang would you mind following the instructions here for the DCO tests to pass? https://github.com/src-d/guide/blob/master/developer-community/fix-DCO.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants