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

Updated setup.py for pip installation #851

Merged
merged 6 commits into from
Aug 22, 2019
Merged

Updated setup.py for pip installation #851

merged 6 commits into from
Aug 22, 2019

Conversation

gramhagen
Copy link
Collaborator

Description

updated PR #639 using new setup.py template based on setuputils

Related Issues

#774

Checklist:

  • I have followed the contribution guidelines and code style for this project.
  • I have added tests covering my contributions.
  • I have updated the documentation accordingly.

@gramhagen gramhagen mentioned this pull request Jun 26, 2019
3 tasks
Copy link
Collaborator

@yueguoguo yueguoguo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!


pip install git+http://github.com/microsoft/recommenders/#egg=pkg&subdirectory=reco_utils

**NOTE** - The pip installation does not install any of the necessary package dependencies, it is expected that conda will be used as shown above to setup the environment for the utilities being used.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure but maybe something like reco_utils[pyspark, gpu, cpu] is useful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, I'll test that out. will need to pull in requirements from the generate_conda script.

Copy link
Collaborator

@miguelgfierro miguelgfierro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just minor comments

'Programming Language :: Python :: 3.7',
],
keywords='recommendations recommenders recommender system engine machine learning python spark gpu',
packages=find_packages(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey can you check whether this takes the tests and the notebooks folder? not sure if we have to do:

    packages=find_packages(
        exclude=[
            "*test*",
            "*notebooks",
        ]
    ),

Copy link
Collaborator Author

@gramhagen gramhagen Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good catch, it does indeed pull in tests and scripts since those dirs have __init__.py files (which they probably shouldn't)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll force the setup to move to the reco_utils folder, which will avoid the top level scripts and tests dirs regardless of where the install is invoked from

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing the pip install
What about automated testing to validate pip install functionality?

reco_utils/setup.py Outdated Show resolved Hide resolved
reco_utils/setup.py Outdated Show resolved Hide resolved
reco_utils/setup.py Outdated Show resolved Hide resolved
@miguelgfierro
Copy link
Collaborator

just FYI @awaemmanuel just did the setup.py in NLP microsoft/nlp-recipes#193

README.md Outdated
@@ -39,6 +39,17 @@ To setup on your local machine:

**NOTE** - The [Alternating Least Squares (ALS)](notebooks/00_quick_start/als_movielens.ipynb) notebooks require a PySpark environment to run. Please follow the steps in the [setup guide](SETUP.md#dependencies-setup) to run these notebooks in a PySpark environment.

## Install this repository via PIP
A [setup.py](reco_utils/setup.py) file is provied in order to simplify the installation of this utilities in this repo from the main directory.
Copy link
Collaborator

@simonzhaoms simonzhaoms Aug 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typos:

A setup.py file is provided in order to simplify the installation of this utilities in this repo from the main directory.

README.md Outdated

It is also possible to install directly from Github.

pip install git+http://github.com/microsoft/recommenders/#egg=pkg&subdirectory=reco_utils
Copy link
Collaborator

@simonzhaoms simonzhaoms Aug 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It failed installing from GitHub directly but succeeded for nlp. I am not sure whether it is the problem of pip to install from pull request. I tried reco_utils:

$ pip install -e git://github.com/microsoft/recommenders.git@refs/pull/851/head#egg=reco_utils\&subdirectory=reco_utils
Obtaining reco_utils from git+git://github.com/microsoft/recommenders.git@refs/pull/851/head#egg=reco_utils&subdirectory=reco_utils
  Cloning git://github.com/microsoft/recommenders.git (to revision refs/pull/851/head) to ./src/reco-utils
  Running command git clone -q git://github.com/microsoft/recommenders.git /home/test/Test/src/reco-utils
  WARNING: Did not find branch or tag 'refs/pull/851/head', assuming revision or ref.
  Running command git fetch -q git://github.com/microsoft/recommenders.git refs/pull/851/head
  Running command git checkout -q ec1ce832b3aaaa2f8889b92d33a64d75981527e6
Installing collected packages: reco-utils
  Found existing installation: reco-utils 2019.6
    Uninstalling reco-utils-2019.6:
      Successfully uninstalled reco-utils-2019.6
  Running setup.py develop for reco-utils
Successfully installed reco-utils
$ ls
src
$ ls src/
pip-delete-this-directory.txt  reco-utils
$ python
Python 3.7.3 (default, Mar 27 2019, 22:11:17) 
[GCC 7.3.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import reco_utils
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'reco_utils'
>>> 

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try using the branch instead of the PR ref? @gramhagen/pip#egg...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simonzhaoms I updated the instructions in the readme this is the format you need:

pip install -e git+https://github.com/microsoft/recommenders/@branch#egg=pkg\&subdirectory=reco_utils

@gramhagen gramhagen merged commit da10819 into staging Aug 22, 2019
@gramhagen gramhagen deleted the gramhagen/pip branch August 22, 2019 00:08
yueguoguo pushed a commit that referenced this pull request Sep 9, 2019
* updating setup.py for pip installability

* updating setup.py to exclude extra dirs and applying black formatting

* black formatting

*  fixing pip install from git links
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.

5 participants