8000 DOC: Add installation mode in How to contribute section · Issue #12815 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content
8000

DOC: Add installation mode in How to contribute section #12815

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.

Sign up for GitHub

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

Jump to bottom
Closed
jmehault opened this issue Dec 18, 2018 · 9 comments · Fixed by #12850
Closed

DOC: Add installation mode in How to contribute section #12815

jmehault opened this issue Dec 18, 2018 · 9 comments · Fixed by #12850
Labels
Documentation Easy Well-defined and straightforward way to resolve help wanted

Comments

@jmehault
Copy link
Contributor

Description

Wanted to start contributing to scikit-learn. Then, I followed the Way to contribute section, but I failed while running the example scripts. I posted a question on stackOverflow to get an answer. The result was I did not get the good installation method.
May be a reminder of Building the library from source + editable installation could be added in the documentation to make a complete recipe.

Steps/Code to Reproduce

Follow actual documentation

Expected Results

Following the "how to contribute" section of the documentation provide working dev environment

Actual Results

Installation method as editable from source is missing to completely work

Versions

scikit-learn v0.20.1
@adrinjalali
Copy link
Member

It's true that those guides are not accurate. I personally do python setup.py install && make inplace before any tests to make sure I have everything in place.

@adrinjalali adrinjalali added Easy Well-defined and straightforward way to resolve Documentation help wanted labels Dec 18, 2018
@jmehault
Copy link
Contributor Author
jmehault commented Dec 19, 2018 •
edited
Loading

May be standard steps should be defined (for each OS ?) so that the development environment is easy to install. I think the perimeter of the guide should be self-consistent. Here are the whole steps I used:

  1. Create an account on GitHub if you do not already have one.
  2. Fork the project repository: click on the ‘Fork’ button near the top of the page. This creates a copy of the code under your account on the GitHub server. For more details on how to fork a repository see this guide.
  3. Clone this copy to your local disk:
    $ git clone git@github.com:YourLogin/scikit-learn.git
  4. Install library in editable mode:
    $ pip install --editable . --prefix=/my/path/to/scikit-learn
  5. Create a branch to hold your changes:
    $ git checkout -b my-feature
  6. Work on this copy, on your computer, using Git to do the version control. When you’re done editing, do:
$ git add modified_files
$ git commit

to record your changes in Git, then push them to GitHub with:
$ git push -u origin my-feature
7. Create a pull request from your fork

Questions:

  • I think the 4th step is common to every OS, right ?
  • May be the PYTHONPATH env. variable must be created/updated to point to /my/path/to/scikit-learn.
  • Should we add the synchronization step from upstream repository (scikit-learn/scikit-learn) ?:
$ git fetch upstream
$ git checkout master
$ git merge upstream/master

@amueller
Copy link
Member

I wouldn't add the prefix, why would you want to do that? But yes, 4 is missing from the docs right now. Interestingly it's in the documentation section:
https://scikit-learn.org/stable/developers/contributing.html#documentation

And in the "building from source" section:
https://scikit-learn.org/stable/developers/advanced_installation.html#install-bleeding-edge

We should probably link to the "building from source" section here?

@jmehault
Copy link
Contributor Author
jmehault commented Dec 20, 2018 •
edited
Loading

Uh, I am adding the prefix to throw the dev repository away from the scikit-learn's stable version. I guess from your answer there is a better a way.

In the guide, I will remove the prefix in the 4th steps and add the link to the "building from source" section for more details. Could be a good first improvement.

@amueller
Copy link
Member

Uh, I am adding the prefix to throw the dev repository away from the scikit-learn's stable version.

Sorry, I'm a bit slow. Can you explain what you mean by that?

@jmehault
Copy link
Contributor Author
jmehault commented Dec 21, 2018 •
edited
Loading

I want to currently work with a stable version of scikit-learn (0.20), but also would like to contribute to the development version. Then I installed 0.20 in a user local directory (~/.local/lib) and the dev one in another one (/other/path). In order to install the dev version in editable mode, I used the --prefix in addition to pip install --editable .. If not, dev version is installed in the same repository than the stable version 0.20. Does it make sense ?

@rth
Copy link
Member
rth commented Dec 21, 2018

Then I installed 0.20 in a user local directory (~/.local/lib) and the dev one in another one (/other/path)

A better way of achieving this can be done with virtual environments: installing the dev version from sources in one environment and the stable version (with pip from PyPi or conda) in another one.

It's generally recommended to avoid manually setting --prefix or PYTHONPATH.

@jmehault jmehault mentioned this issue Dec 21, 2018
Merged
@jmehault
Copy link
Contributor Author

Oh OK! Thanks @rth, I'm gonna try this.

@AbhishekBabuji
Copy link

This thread is actually incredibly helpful.. Easy set of steps - all listed with just the amount of explanation needed to start contributing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation Easy Well-defined and straightforward way to resolve help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[MRG] DOC: add install mode to contribute jmehault/scikit-learn
5 participants
@amueller @rth @adrinjalali @jmehault @AbhishekBabuji
0