Updating the setup file for packaging #38
Conversation
darshshah
commented
Jun 15, 2020
darshshah
commented
- Added LICENSE.txt from parent dir
- Created MANIFEST.in to include license and readme
- Updated setup.py to add additional information
|
Created a whl distribution that can be uploaded to pypi:
Checked the distribution with Twine: $ twine check dist/* |
| data_files=[("ml4ir/build", ["ml4ir/build/Dockerfile"])], | ||
| author_email="searchrelevancyscrumteam@salesforce.com", | ||
| url="https://www.salesforce.com/", | ||
| classifiers=[ |
There was a problem hiding this comment.
are you following a tutorial? Have you done this in the past?
Looks good to me, but I have not release at pip
There was a problem hiding this comment.
Yes, I am following https://packaging.python.org/tutorials/packaging-projects/
| @@ -11,10 +11,10 @@ | |||
| ml4ir can be installed as a pip package by using the following command | |||
There was a problem hiding this comment.
The PR looks good to me. I am approving. I tested with pip install ml4ir, things work!
A personal comment here (feel free to ignore it):
- Consider removing entirely "ml4ir can be installed as a pip package by using the following command" and "This will install ml4ir-0.0.1 (the current version) from PyPI" as they don't add something (since you touched this place of the README)
- Consider adding a sentence below "# ml4ir: Machine Learning Library for Information Retrieval" that says what the package does, like "State-of-the-art implementations of Ranking algorithms based on Tensorflow. Uses tfrecords to optimize for speed and memory. It scales to industry-size datasets out of the box! " Something like this. There is no hint what
ml4irdoes, until the line 105 where I see ranking.
I consider this important in open source. We have only one chance to make a user try it. If you feel this is out of the scope of the story (perhaps you are right), ignore it, I will add a separate story on this.
cc: @lastmansleeping @jakemannix we are now online!!!
We should make some effort to clearly communicate what we do well!
There was a problem hiding this comment.
Yeah, we can add this when we do 0.0.2 release.
| "Programming Language :: Python :: 3 :: Only" | ||
| ], | ||
| include_package_data=True, | ||
| data_files=[("ml4ir/build", ["ml4ir/build/Dockerfile", "ml4ir/build/run_driver.sh"])], |
There was a problem hiding this comment.
should we add the docker-compose.yml here? Or doing pip install is mutually exclusive with docher-compose up. In general, after doing pip install I am not able to run the scripts shown in the remaining of the README. Which makes sense, because the paths indicated there are obsolete. We need to fix this I guess. Disclaimer: I am a docker beginner, I need to learn more about it.
There was a problem hiding this comment.
From what I understand, to use docker we need to git clone the repo and not pip install. @lastmansleeping might have more insights into this
There was a problem hiding this comment.
I don't think there is a need to add docker-compose.yml to setup. We can also pull the Dockerfile now from the virtualenv/pythonpath because we have made sure to add it to the pip installable ml4ir package. So it should be good.
