Repo jbei org changes #5
Conversation
The predict_loading_concentration module was importing itself, which is probably not what we want it doing.
Remove the run.sh and Pipfile for this repo since they are not used. Also, revert the README to its previous state in commit 651b2c3.
The real predict_loading_concentration function that was called in the two predict_loading_concentration.py files was located in __init__.py. This real function was moved to diva_seq_opt/utility. The duplicated predict_loading_concentration.py in diva_seq_opt was deleted, since the same actions are done by the parent directory's predict_loading_concentration.py. The __init__.py was cleared of all text, so it serves only to make diva_seq_opt a module. The main predict_loading_concentration.py was modified to make the call to predict_loading_concentration() in utility.py.
| parser.add_argument('OF' ,type=str, help='Prediction Plot output file, *.png') | ||
| args = parser.parse_args() | ||
| #Load Model | ||
| with open('./diva_seq_opt/model/model30.pkl','rb') as fp: |
There was a problem hiding this comment.
Later on we should change this model name to e.g. model_latest.pkl and always have a copy of the latest model. For example, at the end of the year the model directory will contain:
model_2018.pkl, model_05_2021.pkl, model_12_2021.pkl, model_latest.pkl, where the last two would be the same.
There was a problem hiding this comment.
I very much agree for the future!
There was a problem hiding this comment.
Can you please include a TODO about that before this line?
There was a problem hiding this comment.
I think the path for models should not be among the code but in a separate directory at the same level as notebooks.
|
Also, please check the README, in particular the installation instructions. |
jdmccauley
force-pushed
the
repo_jbei_org_changes
branch
from
b1f7cb8 to
3f04b85
Compare
Moved an import statement in predict_loading_concentration:main from main() to the top. Edited setup.py to call the new predict_loading_concentration. Iterated version. Updated REAMDE to show dependency installation instructions. Finish the last sentence of the README.
jdmccauley
force-pushed
the
repo_jbei_org_changes
branch
from
3f04b85 to
56d43df
Compare
| pip install diva_dna_seq | ||
| The dependencies of this tool can be installed with: | ||
| ``` | ||
| pip install -r requirements.txt |
There was a problem hiding this comment.
What about installing the package using pip? https://pypi.org/project/diva-seq-opt/
That done using setup.py. I can help with that if you'd like.
| @@ -0,0 +1,6 @@ | |||
| #!/bin/sh | |||
|
|
|||
| # Installs the pipenv for running the tool. | |||
There was a problem hiding this comment.
There should be instructions for this in README.
| parser.add_argument('OF' ,type=str, help='Prediction Plot output file, *.png') | ||
| args = parser.parse_args() | ||
| #Load Model | ||
| with open('./diva_seq_opt/model/model30.pkl','rb') as fp: |
There was a problem hiding this comment.
I think the path for models should not be among the code but in a separate directory at the same level as notebooks.
The predict_loading_concentration module was importing itself, which is
probably not what we want it doing.