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

Pip pkg #39

Merged
merged 10 commits into from
Nov 24, 2021
Merged

Pip pkg #39

merged 10 commits into from
Nov 24, 2021

Conversation

andrewkern
Copy link
Member

starting a PR for a packaged up diploshic. take a look @dschride

@dschride
Copy link
Contributor

Just a few problems found. First up, following the instructions from https://test.pypi.org/project/diploSHIC/0.3333/, had this issue:

$ pip install tensorflow
ERROR: Could not find a version that satisfies the requirement tensorflow (from versions: none)
ERROR: No matching distribution found for tensorflow

worked around using conda to install tensorflow and keras.

Next problem:

$ python diploSHIC.py -h
python: can't open file 'diploSHIC.py': [Errno 2] No such file or directory

failed workaround:

$ diploSHIC.py -h
/Users/dan/anaconda3/envs/testdiploshic/bin/diploSHIC.py: line 1: import: command not found
/Users/dan/anaconda3/envs/testdiploshic/bin/diploSHIC.py: line 3: pyExec: command not found
/Users/dan/anaconda3/envs/testdiploshic/bin/diploSHIC.py: line 5: syntax error near unexpected token `('
/Users/dan/anaconda3/envs/testdiploshic/bin/diploSHIC.py: line 5: `    diploShicDir = "/".join(sys.argv[0].split("/")[:-1]) + "/"

worked around with this by setting $PathToDiploSHIC to the result of which diploSHIC.py. So this now works:

python $PathToDiploSHIC/diploSHIC.py -h

Also successfully trained a model after these workarounds.

@andrewkern
Copy link
Member Author

andrewkern commented Nov 22, 2021

ack don't follow those directions! pypi is just copying the readme... instead try this in a clean conda env

python -m pip install --extra-index-url https://test.pypi.org/simple/ diploSHIC==0.33333

after install just type diploSHIC to run the script. the real test is

diploSHIC train diploshic/training/ diploshic/testing/ fooModel --epochs 1

installs and runs on my ubuntu box

@andrewkern
Copy link
Member Author

i've put the pip package on the test pypi server only for now

@dschride
Copy link
Contributor

That is the pip command I used to install. So that's not the issue. Anyway, here is what happens when I do as you say above:

$diploSHIC train $dataPath/training/ $dataPath/testing/ test.mod
-bash: diploSHIC: command not found

$ diploSHIC.py train $dataPath/training/ $dataPath/testing/ test.mod
$ $PathToDiploSHIC/diploSHIC.py: line 1: import: command not found
$PathToDiploSHIC/diploSHIC.py: line 3: pyExec: command not found
$PathToDiploSHIC/diploSHIC.py: line 5: syntax error near unexpected token `('
$PathToDiploSHIC/diploSHIC.py: line 5: `    diploShicDir = "/".join(sys.argv[0].split("/")[:-1]) + "/"'

$ python  $PathToDiploSHIC/diploSHIC.py train $dataPath/training/ $dataPath/testing/ test.mod
Using TensorFlow backend.
loading data now...

(Warnings omitted from output.)

Anyway, seems like the problem could be fixed by adding a shebang to the top of diploSHIC.py. Not sure why/how it works for you!

@dschride
Copy link
Contributor

Oh nevermind, I see the extra 3 in the version number (nice numbering!). Anyway that seems to take care of it.

@andrewkern
Copy link
Member Author

Nice so it's working?

@andrewkern
Copy link
Member Author

I think once we merge this change I'll do a full linting with flake8 so we can set up auto testing on GH

@dschride
Copy link
Contributor

Yep, good to go.

moving stuff

tf2 various updates

version

linting

working on pip

working on pip..

version

still werking

few more paths

what about conda?
@andrewkern
Copy link
Member Author

okay i'm still wrestling with pypi, but i now have a pip package ready to go if you clone and install..

so try something like

conda create -n diploshic python=3.9 --yes
conda activate diploshic
git clone https://github.com/kern-lab/diploSHIC.git
cd diploSHIC 
pip install .

@molpopgen
Copy link
Contributor

You guys may want to take a look at #38 while you're at it--it has been open a long time.

@andrewkern
Copy link
Member Author

@molpopgen do you know anything about manylinux wheels and pypi? I'm trying to get this PR to auto build and deploy and I think it's nearly there...

@molpopgen
Copy link
Contributor

Yeah--see the fwdpy11 actions. But, I think your wheels may not work unless you fix #38, too. Right now, this package is easy to install but very difficult to use end to end.

@molpopgen
Copy link
Contributor

Looking quickly over the PR, you may be missing 3 key files:

Setup.cfg
Pyproject.toml
Requirements.txt

This project is using deprecated/removed keras/tf API routines (unless those fixes are in this pr), so you need the above files to pin the key dependencies. This is what I mean by your wheels will upload but not work for users.

@molpopgen
Copy link
Contributor

So...#38 was closed but this PR doesn't contain the fix. This pr doesnt close that issue without pinning and tests to confirm that the version pinning is correct.

@andrewkern
Copy link
Member Author

This project is using deprecated/removed keras/tf API routines (unless those fixes are in this pr), so you need the above files to pin the key dependencies. This is what I mean by your wheels will upload but not work for users.

Tensorflow API updates have been made so this is now compatible with up-to-date tf versions

@molpopgen
Copy link
Contributor

This project is using deprecated/removed keras/tf API routines (unless those fixes are in this pr), so you need the above files to pin the key dependencies. This is what I mean by your wheels will upload but not work for users.

Tensorflow API updates have been made so this is now compatible with up-to-date tf versions

So that's great, but you will probably still need requirements/setup files to pin tf and/or keras. These dependencies don't seem that dependable, so you probably want to pin to the current major version of both.

@andrewkern
Copy link
Member Author

okay the binary downloads from test.pypi are working just fine from this PR. I'm going to merge this into master and get a versioned release

@andrewkern andrewkern merged commit 2d357d2 into master Nov 24, 2021
@andrewkern andrewkern deleted the pip_pkg branch November 24, 2021 20:29
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.

3 participants