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

Separate Contributing documentation from API how-to #14582

Open
jnothman opened this issue Aug 6, 2019 · 12 comments
Open

Separate Contributing documentation from API how-to #14582

jnothman opened this issue Aug 6, 2019 · 12 comments

Comments

@jnothman
Copy link
Member

@jnothman jnothman commented Aug 6, 2019

https://github.com/scikit-learn/scikit-learn/blob/doc/developers/contributing.rst currently mixes a lot of discussion of how to contribute and how to develop scikit-learn compatible stuff. I think we should separate out "Contributing" from "Developing estimators" (this should become a new file in doc/developers). I'm not sure which comes first, and there is some content that would be hard to place.

This is the current TOC with a suggestion for where each section belongs (up for discussion):

  • Ways to contribute - Contributing
  • Submitting a bug report or a feature request - Contributing
    • How to make a good bug report
  • Contributing code - Contributing
    • How to contribute
    • Pull request checklist
      • Continuous Integration (CI)
      • Stalled pull requests
    • Issues for New Contributors
  • Documentation - Contributing
    • Building the documentation
    • Guidelines for writing documentation
    • Generated documentation on CircleCI
  • Testing and improving test coverage - Developing
    • Writing matplotlib related tests
    • Workflow to improve test coverage
    • Issue Tracker Tags - Contributing - I don't think this belongs here in any case
  • Coding guidelines - Contributing for top matter
    • Input validation - Developing?
    • Random Numbers - Developing
    • Deprecation - Contributing
    • Change the default value of a parameter - Contributing
    • Python versions supported - Contributing
  • Code Review Guidelines - Contributing
  • APIs of scikit-learn objects - Developing
    • Different objects
    • Estimators
      • Instantiation
      • Fitting
      • Estimated Attributes
      • Optional Arguments
      • Pairwise Attributes
  • Rolling your own estimator - Developing
    • get_params and set_params
    • Parameters and init
    • Cloning
    • Pipeline compatibility
    • Estimator types
    • Specific models
    • Estimator Tags
      Reading the existing code base - Contributing
@adrinjalali

This comment has been minimized.

Copy link
Member

@adrinjalali adrinjalali commented Aug 7, 2019

Looks good to me. At least it's a massive improvement.

@glemaitre

This comment has been minimized.

Copy link
Contributor

@glemaitre glemaitre commented Aug 7, 2019

Good to me as well.

@amueller

This comment has been minimized.

Copy link
Member

@amueller amueller commented Aug 7, 2019

I think this is a good first step, but I would like to move one level up and clean up https://scikit-learn.org/dev/developers/index.html

But maybe separating these two is a good first step in cleaning that up.

@arpanchowdhry

This comment has been minimized.

Copy link
Contributor

@arpanchowdhry arpanchowdhry commented Aug 8, 2019

@jnothman I can help with this one. Do we just want to separate the content in to two different files or let me know if there are other improvements that you want to make as part of this change also?

@NicolasHug

This comment has been minimized.

Copy link
Contributor

@NicolasHug NicolasHug commented Aug 8, 2019

@arpanchowdhry a first PR separating the content into 2 different files would be a great start ;)

@sameshl

This comment has been minimized.

Copy link
Contributor

@sameshl sameshl commented Aug 9, 2019

@jnothman looks like the link you posted is broken? Here is the correct one https://github.com/scikit-learn/scikit-learn/blob/master/doc/developers/contributing.rst

I would like to work on this. So as I understand you want the docs to be separated by how you mentioned above?

@arpanchowdhry

This comment has been minimized.

Copy link
Contributor

@arpanchowdhry arpanchowdhry commented Aug 9, 2019

@NicolasHug Thanks. I am on it.

@arpanchowdhry

This comment has been minimized.

Copy link
Contributor

@arpanchowdhry arpanchowdhry commented Aug 9, 2019

@sameshl I am already working on this. Maybe you can working on the further improvements after I am done.

@arpanchowdhry

This comment has been minimized.

Copy link
Contributor

@arpanchowdhry arpanchowdhry commented Aug 9, 2019

@jnothman I used the initial summary as a guideline to separate the sections. This is just an initial version to get feedback. The new file with developing estimators is pretty bare bones and might need more verbiage.

@logan-lieou

This comment has been minimized.

Copy link

@logan-lieou logan-lieou commented Aug 9, 2019

a majority of the :ref:selective <selectiveness> links are dead, for some reason is that intentional or something that also needs to be fixed?

I'm new so I don't really know what's going on and just trying to get involved so if there's anything I can do I want to know.

@amueller

This comment has been minimized.

Copy link
Member

@amueller amueller commented Aug 9, 2019

@logan-lieou sounds like it needs to be fixed. That was supposed to link to the FAQ

thomasjpfan added a commit that referenced this issue Aug 27, 2019
@adrinjalali adrinjalali added this to Design/analysis stage in API and interoperability Oct 22, 2019
@profwacko

This comment has been minimized.

Copy link

@profwacko profwacko commented Oct 26, 2019

I'd like to help out with fixing some of those broken refs, will check out contributing.rst

EDIT: or are they just broken if you try to follow the links on github? Am sort of new to this. It seems okay on the actual documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
API and interoperability
Design/analysis stage
9 participants
You can’t perform that action at this time.