Windows and Linux Script Installers #78
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
caldwella2
added
enhancement
|
Hi @caldwella2, thanks for the PR. Can you please update the PR title with a more descriptive summarization of the change? Thanks! |
noorbuchi
requested changes
Apr 11, 2021
There was a problem hiding this comment.
Thanks for your work on this feature. I have a few small requests before merging this.
- It's important to include documentation on those scripts by updating
README.mdand letting users know that they could either follow the individual commands OR run the scripts you wrote. Additionally, you'll need to include the commands to run the scripts and which one for which operating system. - Since there aren't any changes being made to the python files, please make sure to not include
Pipfle.lockin this pull request - I think it's a good idea to put these scripts in a separate folder that could be called
scripts. This is important to keep the repository organized, especially that new scripts are being added for Docker. We can discuss this further if you have any questions.
Codecov Report
@@ Coverage Diff @@
## master #78 +/- ##
=======================================
Coverage 92.09% 92.09%
=======================================
Files 6 6
Lines 253 253
=======================================
Hits 233 233
Misses 20 20 |
noorbuchi
requested changes
Apr 27, 2021
Comment on lines
+1
to
+17
| #!/bin/bash | ||
| if [ -f "$(eval 'which pip')" ] | ||
| then | ||
| pip_version="pip" | ||
| elif [ -f "$(eval 'which pip3')" ] | ||
| then | ||
| pip_version="pip3" | ||
| else | ||
| $(eval "python3 -m pip install --upgrade pip") | ||
| pip_version="pip" | ||
| fi | ||
|
|
||
| echo $(eval "$pip_version install pipenv -U") | ||
| echo $(eval "cd ..") | ||
| echo $(eval "pipenv install") | ||
| echo $(eval "pipenv run python -m spacy download en_core_web_sm") | ||
| echo $(eval "pipenv run python -m spacy download en_core_web_md") |
Comment on lines
+1
to
+5
|
|
||
|
|
||
| pipenv install | ||
| pipenv run python -m spacy download en_core_web_sm | ||
| pipenv run python -m spacy download en_core_web_md |
There was a problem hiding this comment.
Any status update on the windows script?
There was a problem hiding this comment.
We are currently working on a version that executes the correct commands with the correct versions, yet we are having some issues where there are some dependencies that error out before installing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Together we decided it would be easier to have a script to download all the spacy downloads and pipenv installs in one go rather than having to individually type and run them one at a time.
What is the current behavior?
The user has to individually type/run each command to download spacy and pipenv.
#72
What is the new behavior if this PR is merged?
Pertaining to the operating system they are using, the user can type install.bat or install.sh to install the spacy downloads as well as pipenv installs.
The installs and downloads above are what is inside install.bat for Windows and in install.sh there is an additive of:
Please describe the pull request as one of the following:
Other information
This PR has:
(Other Windows and Linux users tested the installs)
Developers
@PaigeCD @caldwella2 @angusn8 @connellyw @nathandloria