[MRG] DOC add versionadded versionchanged v0.19 by noatamir · Pull Request #16233 · scikit-learn/scikit-learn

noatamir · 2020-01-25T18:42:50Z

finished v0.19.1 (added as main version v0.19 as per Adrin's advice), still working through v0.19

@adrinjalali #wimlds #scikitlearnsprint

adrinjalali

The other changes in that changelog were already covered @noatamir ?

noatamir · 2020-01-29T17:53:31Z

Hi @adrinjalali, so far yes. I'm half-way through checking them all and will finish the rest of the list this Saturday. Thanks for the follow-up.

cmarmo · 2020-02-06T12:53:02Z

Maybe @jnothman could have a look? One approval already and failing checks unrelated to changes. Thanks!

@adrinjalali

Towards scikit-learn#15426 @adrinjalali #wimlds #scikitlearnsprint

noatamir · 2020-02-25T17:15:43Z

@adrinjalali please review new changes
#wimlds

adrinjalali · 2020-02-26T09:52:43Z

You have some linter issues @noatamir :)

noatamir · 2020-02-26T14:43:52Z

of course I do! thanks for catching them, I'll double check..

noatamir · 2020-02-28T13:19:29Z

@adrinjalali can you check again please?

cmarmo · 2020-02-28T13:48:40Z

Hi @noatamir there are still some lines longer than 79 characters.
May I suggest to run flake8 <name-of-the-modified-file> locally in order to check? Thanks for your work!

adrinjalali · 2020-02-28T14:35:14Z

Alternatively, you could run ./build_tools/circle/linting.sh from your sklearn fork see the exact same result as the CI, since running flake8 on the files may give errors which are not related to your PR, and you should ignore them :D

noatamir · 2020-02-28T14:40:18Z

Only one of my files has a flake8 error now, and it's in the code, not where I added docstrings.

sklearn/neighbors/_nearest_centroid.py:143:80: E501 line too long (82 > 79 characters)
sklearn/neighbors/_nearest_centroid.py:145:80: E501 line too long (82 > 79 characters)

Ah just saw @adrinjalali comment. I'll try that too.

noatamir · 2020-02-28T14:47:07Z

passed lint. Let me know if there's any other issue. Thanks for the pointers!

adrinjalali · 2020-03-02T10:16:33Z

@cmarmo I can't easily figure the indentation issue here. Is it the same issue as the other PR we had?

cmarmo · 2020-03-02T12:12:21Z

@cmarmo I can't easily figure the indentation issue here. Is it the same issue as the other PR we had?

I think so... @noatamir, could you add a blank line before the .. versionadded or .. versionchanged directives? This will improve readability and probably fix the sphinx warning. Thanks!

cmarmo

Apart those minor comments. Thanks @noatamir for your work.

cmarmo · 2020-03-11T13:08:33Z

sklearn/decomposition/_lda.py

    ----------
    n_components : int, optional (default=10)
        Number of topics.
+        .. versionchanged:: 0.19


Suggested change

.. versionchanged:: 0.19

.. versionchanged:: 0.19

This change will fix the sphinx warning.

cmarmo · 2020-03-11T13:09:19Z

sklearn/feature_extraction/_hash.py

        When True, an alternating sign is added to the features as to
        approximately conserve the inner product in the hashed space even for
        small n_features. This approach is similar to sparse random projection.
+    .. versionchanged:: 0.19


Suggested change

.. versionchanged:: 0.19

.. versionchanged:: 0.19

just for consistency

cmarmo · 2020-03-11T13:09:45Z

sklearn/multiclass.py

        Returns
        -------
        Y : array-like of shape (n_samples, n_classes)
+            .. versionchanged:: 0.19


Suggested change

.. versionchanged:: 0.19

.. versionchanged:: 0.19

just for consistency

cmarmo · 2020-03-11T13:13:05Z

.gitignore

 sklearn/feature_selection/rfe.py
 sklearn/feature_selection/univariate_selection.py
 sklearn/feature_selection/variance_threshold.py
+Pipfile


Could you please remove Pipfile from .gitignore? The file is created by your virtual environment. To avoid this problem you can create the virtual environment files outside the development directory.

cmarmo · 2020-03-31T17:30:01Z

Hi @noatamir, hope you are fine ... one approval already for this PR. Maybe you can find some time to fix the last issues so it can be merged?

noatamir · 2020-04-05T10:14:46Z

@cmarmo Thanks for the detailed comments. I made all the changes you mentioned.

Please excuse the late reply, I was a bit derailed by COVID-19.

cmarmo · 2020-04-05T12:35:06Z

Please excuse the late reply, I was a bit derailed by COVID-19.

No worries, we are all a bit derailed those days...

CI is failing as reported in #16545: syncing with master will solve the problem.
Maybe @NicolasHug or @thomasjpfan are available for a final review? Thanks!

thomasjpfan

Thank you for the PR @noatamir !

sklearn/multiclass.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

cmarmo · 2020-04-08T13:04:17Z

Hi @noatamir , could you please sync with upstream/master? This will fix the failing CI. Thanks a lot!

…with_versionadded_versionchanged_for_v0.19

noatamir · 2020-04-14T20:39:40Z

@cmarmo done =)

cmarmo · 2020-04-14T21:50:55Z

Thanks @noatamir! All green! Maybe @NicolasHug is listening? This PR is ready to merge I think...

@adrinjalali

* added v0.19.1 and wip v0.19 * finished adding vchanged strings for v0.19 Towards scikit-learn#15426 @adrinjalali #wimlds #scikitlearnsprint * fixing linter issues * caught line issues with flake8 * caught the last line issue * added lines and cleaned gtiignore * Update sklearn/multiclass.py Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com> * Update sklearn/multiclass.py Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com> Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com>

added v0.19.1 and wip v0.19

7b3abb5

adrinjalali approved these changes Jan 28, 2020

View reviewed changes

cmarmo added the Waiting for Reviewer label Feb 14, 2020

finished adding vchanged strings for v0.19

167bb1a

Towards scikit-learn#15426 @adrinjalali #wimlds #scikitlearnsprint

fixing linter issues

5acb200

caught line issues with flake8

6108347

caught the last line issue

09f2393

cmarmo approved these changes Mar 11, 2020

View reviewed changes

added lines and cleaned gtiignore

50dd91c

noatamir changed the title ~~[WIP] DOC add versionadded versionchanged v0.19~~ [MRG] DOC add versionadded versionchanged v0.19 Apr 5, 2020

thomasjpfan reviewed Apr 5, 2020

View reviewed changes

sklearn/multiclass.py Outdated Show resolved Hide resolved

sklearn/multiclass.py Outdated Show resolved Hide resolved

noatamir and others added 2 commits April 6, 2020 09:53

Update sklearn/multiclass.py

1f76c1a

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

Update sklearn/multiclass.py

ee5d324

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

thomasjpfan approved these changes Apr 6, 2020

View reviewed changes

cmarmo removed the Waiting for Reviewer label Apr 6, 2020

Merge remote-tracking branch 'upstream/master' into Label_docstrings_…

f518577

…with_versionadded_versionchanged_for_v0.19

adrinjalali merged commit 5e2d74b into scikit-learn:master Apr 15, 2020

Uh oh!

Conversation

noatamir commented Jan 25, 2020

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

noatamir commented Jan 29, 2020

Uh oh!

cmarmo commented Feb 6, 2020

Uh oh!

noatamir commented Feb 25, 2020

Uh oh!

adrinjalali commented Feb 26, 2020

Uh oh!

noatamir commented Feb 26, 2020

Uh oh!

noatamir commented Feb 28, 2020

Uh oh!

cmarmo commented Feb 28, 2020

Uh oh!

adrinjalali commented Feb 28, 2020

Uh oh!

noatamir commented Feb 28, 2020

Uh oh!

noatamir commented Feb 28, 2020

Uh oh!

adrinjalali commented Mar 2, 2020

Uh oh!

cmarmo commented Mar 2, 2020

Uh oh!

cmarmo left a comment

Choose a reason for hiding this comment

Uh oh!

cmarmo Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

cmarmo Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

cmarmo Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

cmarmo Mar 11, 2020

Choose a reason for hiding this comment

Uh oh!

cmarmo commented Mar 31, 2020

Uh oh!

noatamir commented Apr 5, 2020

Uh oh!

cmarmo commented Apr 5, 2020

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cmarmo commented Apr 8, 2020

Uh oh!

noatamir commented Apr 14, 2020

Uh oh!

cmarmo commented Apr 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants