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

Add a setup.py #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add a setup.py #120

wants to merge 2 commits into from

Conversation

giamic
Copy link

@giamic giamic commented Dec 6, 2022

Add a setup.py file to the repo. It allows people to install AugmentedNet by simply running

pip install git+https://github.com/napulen/AugmentedNet

Of course, this also comes with a better integration for conda's environment.yml files.

I have been working with https://github.com/MarkGotham/When-in-Rome that depends on AugmentedNet and I was having problems to install AugmentedNet so that's why I came up with this.

One thing to notice is that there is some information to be added such as author name and email which will probably require some modifications.

@napulen
Copy link
Owner

napulen commented Jan 3, 2023

Hi @giamic, first of all, thank you for writing this setup.py and sorry for the delayed reply!

I tried the setup and it unfortunately lead to some issues when trying to run python -m AugmentedNet.inference, meaning that dependencies are not properly handled. Sadly, I have also seen happening with the requirements.txt files (although the current one should still work properly). It seems python dependencies are a moving target and one quickly runs into "dependency hell". Nowadays, when I am installing on a new development environment, I use a conda environment.yml, which yields more reliable results.

Here is the environment.yml that I am using today:

name: augmentednet-gpu
channels:
  - anaconda
  - conda-forge
  - defaults
dependencies:
  - _libgcc_mutex=0.1=conda_forge
  - _openmp_mutex=4.5=2_gnu
  - bzip2=1.0.8=h7b6447c_0
  - ca-certificates=2022.07.19=h06a4308_0
  - certifi=2022.6.15=py310h06a4308_0
  - cudatoolkit=11.2.2=hbe64b41_11
  - cudnn=8.1.0.77=h90431f1_0
  - ld_impl_linux-64=2.38=h1181459_1
  - libffi=3.3=he6710b0_2
  - libgcc-ng=12.2.0=h65d4601_19
  - libgomp=12.2.0=h65d4601_19
  - libstdcxx-ng=12.2.0=h46fd767_19
  - libuuid=1.0.3=h7f8727e_2
  - ncurses=6.3=h5eee18b_3
  - openssl=1.1.1q=h7f8727e_0
  - pip=22.1.2=py310h06a4308_0
  - python=3.10.4=h12debd9_0
  - readline=8.1.2=h7f8727e_1
  - setuptools=61.2.0=py310h06a4308_0
  - sqlite=3.39.2=h5082296_0
  - tk=8.6.12=h1ccaba5_0
  - tzdata=2022a=hda174b7_0
  - wheel=0.37.1=pyhd3eb1b0_0
  - xz=5.2.5=h7f8727e_1
  - zlib=1.2.12=h7f8727e_2
  - pip:
    - absl-py==1.3.0
    - alembic==1.8.1
    - asttokens==2.2.1
    - astunparse==1.6.3
    - backcall==0.2.0
    - cachetools==5.2.0
    - chardet==5.1.0
    - charset-normalizer==2.1.1
    - click==8.1.3
    - cloudpickle==2.2.0
    - comm==0.1.2
    - contourpy==1.0.6
    - cycler==0.11.0
    - databricks-cli==0.17.4
    - debugpy==1.6.4
    - decorator==5.1.1
    - docker==6.0.1
    - entrypoints==0.4
    - executing==1.2.0
    - flask==2.2.2
    - flatbuffers==22.12.6
    - fonttools==4.38.0
    - gast==0.4.0
    - gitdb==4.0.10
    - gitpython==3.1.29
    - google-auth==2.15.0
    - google-auth-oauthlib==0.4.6
    - google-pasta==0.2.0
    - greenlet==2.0.1
    - grpcio==1.51.1
    - gunicorn==20.1.0
    - h5py==3.7.0
    - idna==3.4
    - importlib-metadata==5.1.0
    - ipykernel==6.19.2
    - ipython==8.7.0
    - itsdangerous==2.1.2
    - jedi==0.18.2
    - jinja2==3.1.2
    - joblib==1.2.0
    - jupyter-client==7.4.8
    - jupyter-core==5.1.0
    - keras==2.11.0
    - kiwisolver==1.4.4
    - libclang==14.0.6
    - llvmlite==0.39.1
    - mako==1.2.4
    - markdown==3.4.1
    - markupsafe==2.1.1
    - matplotlib==3.6.2
    - matplotlib-inline==0.1.6
    - mlflow==2.0.1
    - more-itertools==9.0.0
    - music21==6.7.1
    - nest-asyncio==1.5.6
    - numba==0.56.4
    - numpy==1.23.5
    - oauthlib==3.2.2
    - opt-einsum==3.3.0
    - packaging==21.3
    - pandas==1.5.2
    - parso==0.8.3
    - pexpect==4.8.0
    - pickleshare==0.7.5
    - pillow==9.3.0
    - platformdirs==2.6.0
    - prompt-toolkit==3.0.36
    - protobuf==3.19.6
    - psutil==5.9.4
    - ptyprocess==0.7.0
    - pure-eval==0.2.2
    - pyarrow==10.0.1
    - pyasn1==0.4.8
    - pyasn1-modules==0.2.8
    - pygments==2.13.0
    - pyjwt==2.6.0
    - pyparsing==3.0.9
    - python-dateutil==2.8.2
    - pytz==2022.6
    - pyyaml==6.0
    - pyzmq==24.0.1
    - querystring-parser==1.2.4
    - requests==2.28.1
    - requests-oauthlib==1.3.1
    - rsa==4.9
    - scikit-learn==1.2.0
    - scipy==1.9.3
    - seaborn==0.12.1
    - shap==0.41.0
    - six==1.16.0
    - slicer==0.0.7
    - smmap==5.0.0
    - sqlalchemy==1.4.44
    - sqlparse==0.4.3
    - stack-data==0.6.2
    - tabulate==0.9.0
    - tensorboard==2.11.0
    - tensorboard-data-server==0.6.1
    - tensorboard-plugin-wit==1.8.1
    - tensorflow==2.11.0
    - tensorflow-estimator==2.11.0
    - tensorflow-io-gcs-filesystem==0.28.0
    - termcolor==2.1.1
    - threadpoolctl==3.1.0
    - tornado==6.2
    - tqdm==4.64.1
    - traitlets==5.7.0
    - typing-extensions==4.4.0
    - urllib3==1.26.13
    - wcwidth==0.2.5
    - webcolors==1.12
    - websocket-client==1.4.2
    - werkzeug==2.2.2
    - wrapt==1.14.1
    - zipp==3.11.0

@napulen
Copy link
Owner

napulen commented Jan 3, 2023

Furthermore, the way I understand this pull request, the intention here is to provide a setup so that the repository can be installed remotely, perhaps as a dependency? Particularly, I'm curious about this file and whether the intention is to add AugmentedNet here?

Please let me know if you have trouble working with this repository and I'll do my best to unblock any issues, particularly if you are running experiments or retraining the network. Thanks again!

@giamic
Copy link
Author

giamic commented Jan 3, 2023

Hi Nestor, thanks for the reply!

You are absolutely right about my intention, I do want to add AugmentedNet as a dependency to WhenInRome and this is why I have come up with this setup.py.

Regarding the dependencies, I think that the best practice for setup.py files is to list the least restrictive dependencies possible. This is to avoid issues when installing several different packages requiring slightly different versions (imagine what would happen if AugmentedNet required 3.6.2 and WhenInRome 3.6.1: conda would have problems solving the environment, while most probably both packages would work with whatever version of matplotlib 3). This is my go-to approach with conda as well, where I list only the dependencies that are explicitly imported by the code with the least restrictive version possible and let conda handle all the internal dependencies.

I am on a different computer now and I don't have the AugmentedNet repo cloned here. Let me check tonight when I go home and verify what I can do to fix the error you're having.

@napulen
Copy link
Owner

napulen commented Jan 4, 2023

I understand the idea of the least number of dependencies possible. With tensorflow, I have unfortunately ran into the situation of letting pip resolve the internal dependencies, succeeding to install, and then failing (throwing exceptions) as soon as tensorflow is imported in a python module. I remember this happening particularly with protobuf and joblib. That's why I opted for the "full list" of dependencies, as seen in the requirements.txt. Given that said, this setup.py doesn't hurt the repo and if it helps others to make it available, I'll merge the request.

A couple of other notes:

  • If the intention is to use AugmentedNet inside a github actions workflow, I suggest looking at the workflow here: https://github.com/napulen/AugmentedNet/blob/main/.github/workflows/main.yml It installs/runs AugmentedNet through the requirements.txt file included in the repo. I intend to maintain that file operational. I can't promise the same for the setup.py
  • Related to the previous point, I was looking at the github actions in When-in-Rome, it seems to mostly be running the unit tests in Tests/, however, none of these tests seems to be using AugmentedNet modules. Are we sure AugmentedNet is needed as a dependency? Just making sure :). Perhaps you have plans to add more tests with it, just bringing up that it doesn't seem to be necessary at the moment.

Tagging @MarkGotham on this, in case he has additional input.

@napulen
Copy link
Owner

napulen commented Jan 4, 2023

One of the unit tests of the repo is to require black -l 79 formatting in all python sources. I took the liberty of committing this change. If you can confirm this setup.py works for your purposes, I have no objections to merging it.

Thanks!

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.

2 participants