[MRG + 2] DOC adding :user: role to whats_new by dalmia · Pull Request #7818 · scikit-learn/scikit-learn

dalmia · 2016-11-03T13:18:15Z

Reference Issue

Addresses Issue #7748

What does this implement/fix? Explain your changes.

Added the :user: role to whats_new.rst for all contributors having a github url in the current version of whats_new.rst.

Any other comments?

Wrote a script for automating the process. No changes made for contributors not having the github URL linked.

maniteja123 · 2016-11-03T15:47:01Z

Hi, I suppose there is a merge conflict since whats new has been modified in master. It might be best to rebase and resolve the conflicts. This is the reason for failure of appveyor build.

dalmia · 2016-11-03T15:55:30Z

Hi @maniteja123, thank you for your response. Since the issue needs whats_new to be modified, I suppose the rebase can be done only by the maintainers?

maniteja123 · 2016-11-03T20:13:17Z

Hi, it is possible for you to resolve conflicts and push the updated ones. The link under rebase your branch on master in docs gives an example. Hope it helps.

jnothman · 2016-11-03T23:13:23Z

The appveyor failure likely is unrelated. You'll need to rebase/merge frequently on this PR :/

jnothman

Don't worry about rebasing until the rest looks right.

jnothman · 2016-11-03T23:13:44Z

doc/sphinxext/sphinx_issues.py

-try:
-    from docutils import nodes, utils
-except ImportError:
-    # Load lazily so that test-sphinxext does not require docutils dependency


You need to keep this variation

jnothman · 2016-11-03T23:14:45Z

doc/whats_new.rst

-   *  96  `Lars Buitinck`_
-   *  88  `Jaques Grobler`_
+   *  96  :user:`Lars Buitinck<larsmans>`
+   *  88  :user:`Jaques Grobler<jaquesgrobler/scikit-learn/wiki/Jaques-Grobler>`


In cases like this you've over-generated.

jnothman · 2016-11-03T23:16:13Z

doc/whats_new.rst


 .. _Scott White: https://twitter.com/scottblanc

-.. _Jaques Grobler: https://github.com/jaquesgrobler/scikit-learn/wiki/Jaques-Grobler


But this should probably be just https://github.com/jaquesgrobler/ anyway.

dalmia · 2016-11-04T00:05:00Z

I was extracting the part following 'http://github.com/' when I was parsing the URL. This particular author has the URL mentioned in the original whats_new as :

.. _Jaques Grobler: https://github.com/jaquesgrobler/scikit-learn/wiki/Jaques-Grobler

Sorry, I didn't anticipate there could be such anomalies. I'll make the changes and look for any other such issues.

dalmia · 2016-11-04T00:42:57Z

@maniteja123 Thank you for the link. It worked like a charm.
I did resolve the conflicts. I hope all the checks pass now!

jnothman · 2016-11-04T02:37:37Z

Thanks, this is much better, but looking at this I think core devs, at least, if not other regular contributors, should switch back to the old format.

Team members have the following logins: agramfort AlexanderFabisch alextp amueller arjoly bdholt1 bthirion chrisfilo cournape duchesnay dwf fabianp GaelVaroquaux glouppe jakevdp jaquesgrobler jmetzen jnothman kastnerkyle larsmans lesteve lucidfrontier45 mblondel MechCoder ndawe NelleV ogrisel paolo-losi pprett raghavrv tomdlt

Or you can switch back for all people whose user appears in the log more than 10 times or something.

Another thing we should do is try to keep line length to at most 80 chars maximum for lines that are changed.

dalmia · 2016-11-04T02:47:34Z

I'll start working on that then!

dalmia · 2016-11-04T08:11:24Z

When I check for the number of times user appears in the log, it also includes those cases where they have been mentioned in the list of contributors after the log for each version. Would it be more appropriate then to set the threshold at 15?

jnothman · 2016-11-04T08:12:08Z

Make it 20. It's immaterial.

On 4 November 2016 at 19:11, Aman Dalmia notifications@github.com wrote:

When I check for the number of times user appears in the log, it also
includes those cases where they have been mentioned in the list of
contributors after the log for each version. Would it be more appropriate
then to set the threshold at 15?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#7818 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEz6xLuAoKXnoT2jP4RPLqUVdYJlK9Uks5q6uiugaJpZM4KoWBZ
.

dalmia · 2016-11-04T08:32:29Z

@jnothman Actually the number of core devs having their github URLs linked was only 5 so I changed them manually. Let me know if this works or should I do this for all those having more than 20 commits.

TomDLT · 2016-11-04T15:05:53Z

reverting to the old format for core devs

What about us? ;)

Nelle Varoquaux
Tom Dupre la Tour

should I do this for all those having more than 20 commits.

why not

dalmia · 2016-11-04T15:31:55Z

@TomDLT I had assumed that the list of usernames provided were case sensitive. My bad. I made a few changes to the script and now the doc has been updated for all the core devs.

dalmia · 2016-11-04T19:08:19Z

@jnothman Please let me know if this has any more issues.

jnothman

Otherwise LGTM

jnothman · 2016-11-05T10:50:01Z

doc/whats_new.rst


-.. _Dylan Werner-Meier: https://github.com/unautre
+.. _Nelle Varoquaux: https://github.com/nellev



While we're at it, could you please remove all the blank lines in this target URL listing?

jnothman · 2016-11-05T10:50:59Z

doc/whats_new.rst

   - New :class:`mixture.GaussianMixture` and :class:`mixture.BayesianGaussianMixture`
     replace former mixture models, employing faster inference
-     for sounder results. :issue:`7295` by `Wei Xue`_ and `Thierry Guillemot`_.
+     for sounder results. :issue:`7295` by :user:`Wei Xue<xuewei4d>` and :user:`Thierry Guillemot<tguillemot>`.


This line is very long

jnothman · 2016-11-05T10:51:05Z

doc/whats_new.rst

   - Random forest, extra trees, decision trees and gradient boosting estimator
     accept the parameter ``min_samples_split`` and ``min_samples_leaf``
-     provided as a percentage of the training samples. By `yelite`_ and `Arnaud Joly`_.
+     provided as a percentage of the training samples. By :user:`yelite<yelite>` and `Arnaud Joly`_.


This line is very long

jnothman · 2016-11-05T10:51:14Z

doc/whats_new.rst

     with ``np.float32`` and ``np.float64`` input data without converting it.
     This allows to reduce the memory consumption by using ``np.float32``.
-     :issue:`6846` by `Sebastian Säger`_ and `YenChen Lin`_.
+     :issue:`6846` by :user:`Sebastian Säger<ssaeger>` and :user:`YenChen Lin<yenchenlin>`.


This line is very long

jnothman · 2016-11-05T10:51:19Z

doc/whats_new.rst


   - :class:`feature_extraction.FeatureHasher` now accepts string values.
-     :issue:`6173` by `Ryad Zenine`_ and `Devashish Deshpande`_.
+     :issue:`6173` by :user:`Ryad Zenine<ryadzenine>` and :user:`Devashish Deshpande<dsquareindia>`.


This line is very long

jnothman · 2016-11-05T10:52:16Z

doc/whats_new.rst

     of the impurity improvement during the tree growth. The proxy quantity is
     such that the split that maximizes this value also maximizes the impurity
-     improvement. By `Arnaud Joly`_, `Jacob Schreiber`_ and `Gilles Louppe`_.
+     improvement. By `Arnaud Joly`_, :user:`Jacob Schreiber<jmschrei>` and `Gilles Louppe`_.


This line is very long

jnothman · 2016-11-05T10:52:27Z

doc/whats_new.rst


    - Fixed bug :issue:`5495` when
-      doing OVR(SVC(decision_function_shape="ovr")). Fixed by `Elvis Dohmatob`_.
+      doing OVR(SVC(decision_function_shape="ovr")). Fixed by :user:`Elvis Dohmatob<dohmatob>`.


This line is very long

jnothman · 2016-11-05T10:52:31Z

doc/whats_new.rst


    - :class:`cluster.KMeans` re-runs cluster-assignments in case of non-convergence,
-      to ensure consistency of ``predict(X)`` and ``labels_``. By `Vighnesh Birodkar`_.
+      to ensure consistency of ``predict(X)`` and ``labels_``. By :user:`Vighnesh Birodkar<vighneshbirodkar>`.


This line is very long

jnothman · 2016-11-05T10:52:37Z

doc/whats_new.rst


   - ``DictVectorizer`` can now perform ``fit_transform`` on an iterable in a
-     single pass, when giving the option ``sort=False``. By `Dan Blanchard`_.
+     single pass, when giving the option ``sort=False``. By :user:`Dan Blanchard<dan-blanchard>`.


This line is very long

jnothman · 2016-11-05T10:52:44Z

doc/whats_new.rst


  - Fixed handling of the ``p`` parameter of the Minkowski distance that was
-    previously ignored in nearest neighbors models. By `Nikolay Mayorov`_.
+    previously ignored in nearest neighbors models. By :user:`Nikolay Mayorov<nmayorov>`.


This line is very long

jnothman · 2016-11-05T11:09:13Z

If you wouldn't mind, I've checked for names without link target definitions and the opposite and found a few issues that need to be fixed up:

Hannah Wallach is mentioned in the text, but not linked (even though a link target is defined at the end of the file)
Johannes Schönberger is mentioned in the text, but not linked, as above
Federico Vaggi is mentioned in the text, but the link is defined as .. _@FedericoV for unknown reason
Jon Crall and Ekaterina Krivich recently contributed (you'll need to update master) but didn't define link targets. They should be github users Erotemic and kiote respectively.
I'm not sure if these link names are case sensitive, but if they are, Jake VanderPlas should change to Jake Vanderplas when it appears as the link name.
Ron Weiss should be defined at the end of the file, as http://ee.columbia.edu/~ronw.

dalmia · 2016-11-05T14:20:40Z

Okay! I'll try to complete them by tomorrow.

jnothman · 2016-11-06T10:13:36Z

Thank you so much!

dalmia · 2016-11-07T04:28:41Z

Thank you :)

raghavrv · 2016-11-07T21:14:29Z

Could you fix conflicts with master?

jnothman · 2016-11-07T23:29:21Z

(That's why we have to merge soon. Perhaps the merger should just solve
conflicts when it comes down to it.)

On 8 November 2016 at 08:14, Raghav RV notifications@github.com wrote:

Could you fix conflicts with master?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7818 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEz6_EQImmf4PegwROg-DLA8MIWtAvsks5q75S2gaJpZM4KoWBZ
.

dalmia · 2016-11-08T09:28:19Z

@raghavrv Please have a look now. I would request you to review it as it updates really quickly in the master branch. Thank you.

dalmia · 2016-11-08T13:50:31Z

@jnothman The error log says that the build execution time reached the maximum allowed time. Please let me know if I should be doing anything in this regard.

jnothman · 2016-11-08T20:23:29Z

No, that's just the nature of using public continuous integration services.

raghavrv

Minor comments otherwise LGTM. Thanks!

raghavrv · 2016-11-08T20:34:47Z

doc/whats_new.rst

 Garrett-R, Gilles Louppe, gpassino, gwulfs, Hampus Bengtsson, Hamzeh Alsalhi,
 Hanna Wallach, Harry Mavroforakis, Hasil Sharma, Helder, Herve Bredin,
 Hsiang-Fu Yu, Hugues SALAMIN, Ian Gilmore, Ilambharathi Kanniah, Imran Haque,
-isms, Jake VanderPlas, Jan Dlabal, Jan Hendrik Metzen, Jatin Shah, Javier López


I think @jakevdp prefers it this way ;)

raghavrv · 2016-11-08T20:35:10Z

doc/whats_new.rst

   - Support unseen labels :class:`preprocessing.LabelBinarizer` to restore
     the default behavior of 0.14.1 for backward compatibility. By
-     `Hamzeh Alsalhi`_.
+     :user:`Hamzeh Alsalhi<hamsal>`.


space after name

raghavrv · 2016-11-08T20:35:36Z

doc/whats_new.rst

 *  21	Maheshakya Wijewardena
 *  21	Brooke Osborn
 *  21	Hamzeh Alsalhi
-*  21	Jake VanderPlas


Same comment as above...

raghavrv · 2016-11-08T20:35:52Z

doc/whats_new.rst


   - Hyperlinks to documentation in example code on the website by
-     `Martin Luessi`_.
+     :user:`Martin Luessi<mluessi>`.


Space after name

raghavrv · 2016-11-08T20:36:06Z

doc/whats_new.rst

 * 102  Noel Dawe
 *  99  Kemal Eren
 *  79  Joel Nothman
- *  75  Jake VanderPlas


Same comment as above

dalmia · 2016-11-09T00:50:14Z

@raghavrv Please let me know if this works now. Thanks.

raghavrv · 2016-11-09T01:00:55Z

Thanks. Will merge once the CIs pass...

raghavrv · 2016-11-09T01:39:47Z

In it goes! Thanks @dalmia

jnothman · 2016-11-09T02:05:57Z

Yay for less finicky changelog management!

dalmia · 2016-11-09T02:37:22Z

Thank you @jnothman, @raghavrv. My first ever PR merged. Feels great :)

jnothman · 2016-11-09T05:55:41Z

:) Thank you.

* DOC adding :user: role to whats_new * DOC correcting the improper username * DOC adding exceptions for ImportError in sphinx_issues.py * DOC added import for split_explicit_title * DOC reverting to the old format for core devs * DOC reverting changes for more users * DOC removing extra lines and blank lines * DOC Adding missing link target for user * DOC fixing more links * DOC adding missing links for new contributions * DOC adding space before brackets in the user links * DOC reverting a wrong change * DOC adding final changes

* tag '0.18.1': (144 commits) skip tree-test on 32bit do the warning test as we do it in other places. Replase assert_equal by assert_almost_equal in cosine test version bump 0.18.1 fix merge conflict mess in whatsnew add the python2.6 warning to 0.18.1 fix learning_curve test that I messed up in cherry-picking the "reentrant cv" PR. sync whatsnew with master [MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (scikit-learn#7553) FIX scikit-learn#6420: Cloning decision tree estimators breaks criterion objects (scikit-learn#7680) Add whats new entry for scikit-learn#6282 (scikit-learn#7629) [MGR + 2] fix selectFdr bug (scikit-learn#7490) fixed whatsnew cherry-pick mess (somewhat) [MRG + 2] FIX LogisticRegressionCV to correctly handle string labels (scikit-learn#5874) [MRG + 2] Fixed parameter setting in SelectFromModel (scikit-learn#7764) [MRG+2] DOC adding separate `fit()` methods (and docstrings) for DecisionTreeClassifier and DecisionTreeRegressor (scikit-learn#7824) Fix docstring typo (scikit-learn#7844) n_features --> n_components [MRG + 1] DOC adding :user: role to whats_new (scikit-learn#7818) [MRG+1] label binarizer not used consistently in CalibratedClassifierCV (scikit-learn#7799) DOC : fix docstring of AIC/BIC in GMM ...

* releases: (144 commits) skip tree-test on 32bit do the warning test as we do it in other places. Replase assert_equal by assert_almost_equal in cosine test version bump 0.18.1 fix merge conflict mess in whatsnew add the python2.6 warning to 0.18.1 fix learning_curve test that I messed up in cherry-picking the "reentrant cv" PR. sync whatsnew with master [MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (scikit-learn#7553) FIX scikit-learn#6420: Cloning decision tree estimators breaks criterion objects (scikit-learn#7680) Add whats new entry for scikit-learn#6282 (scikit-learn#7629) [MGR + 2] fix selectFdr bug (scikit-learn#7490) fixed whatsnew cherry-pick mess (somewhat) [MRG + 2] FIX LogisticRegressionCV to correctly handle string labels (scikit-learn#5874) [MRG + 2] Fixed parameter setting in SelectFromModel (scikit-learn#7764) [MRG+2] DOC adding separate `fit()` methods (and docstrings) for DecisionTreeClassifier and DecisionTreeRegressor (scikit-learn#7824) Fix docstring typo (scikit-learn#7844) n_features --> n_components [MRG + 1] DOC adding :user: role to whats_new (scikit-learn#7818) [MRG+1] label binarizer not used consistently in CalibratedClassifierCV (scikit-learn#7799) DOC : fix docstring of AIC/BIC in GMM ... Conflicts: removed sklearn/externals/joblib/__init__.py sklearn/externals/joblib/_parallel_backends.py sklearn/externals/joblib/testing.py

* dfsg: (144 commits) skip tree-test on 32bit do the warning test as we do it in other places. Replase assert_equal by assert_almost_equal in cosine test version bump 0.18.1 fix merge conflict mess in whatsnew add the python2.6 warning to 0.18.1 fix learning_curve test that I messed up in cherry-picking the "reentrant cv" PR. sync whatsnew with master [MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (scikit-learn#7553) FIX scikit-learn#6420: Cloning decision tree estimators breaks criterion objects (scikit-learn#7680) Add whats new entry for scikit-learn#6282 (scikit-learn#7629) [MGR + 2] fix selectFdr bug (scikit-learn#7490) fixed whatsnew cherry-pick mess (somewhat) [MRG + 2] FIX LogisticRegressionCV to correctly handle string labels (scikit-learn#5874) [MRG + 2] Fixed parameter setting in SelectFromModel (scikit-learn#7764) [MRG+2] DOC adding separate `fit()` methods (and docstrings) for DecisionTreeClassifier and DecisionTreeRegressor (scikit-learn#7824) Fix docstring typo (scikit-learn#7844) n_features --> n_components [MRG + 1] DOC adding :user: role to whats_new (scikit-learn#7818) [MRG+1] label binarizer not used consistently in CalibratedClassifierCV (scikit-learn#7799) DOC : fix docstring of AIC/BIC in GMM ...

* DOC adding :user: role to whats_new * DOC correcting the improper username * DOC adding exceptions for ImportError in sphinx_issues.py * DOC added import for split_explicit_title * DOC reverting to the old format for core devs * DOC reverting changes for more users * DOC removing extra lines and blank lines * DOC Adding missing link target for user * DOC fixing more links * DOC adding missing links for new contributions * DOC adding space before brackets in the user links * DOC reverting a wrong change * DOC adding final changes

DOC adding :user: role to whats_new

6dedda2

jnothman requested changes Nov 3, 2016

View reviewed changes

jnothman reviewed Nov 3, 2016

View reviewed changes

dalmia added 3 commits November 4, 2016 05:43

DOC correcting the improper username

a6b8bd7

DOC adding exceptions for ImportError in sphinx_issues.py

ad1905f

Merging the upstream repo with the local branch to resolve conflicts

c026be7

DOC added import for split_explicit_title

859249d

DOC reverting to the old format for core devs

e6377a3

DOC reverting changes for more users

08d7bb5

jnothman requested changes Nov 5, 2016

View reviewed changes

dalmia added 3 commits November 7, 2016 05:40

DOC removing extra lines and blank lines

4669eb3

DOC Adding missing link target for user

cc90fcb

DOC fixing more links

a15bfb3

dalmia mentioned this pull request Nov 7, 2016

[MRG+2] DOC adding separate fit() methods (and docstrings) for DecisionTreeClassifier and DecisionTreeRegressor #7824

Merged

DOC resolving conflicts with master

61b1a75

jnothman approved these changes Nov 8, 2016

View reviewed changes

raghavrv suggested changes Nov 8, 2016

View reviewed changes

DOC adding final changes

408a8af

raghavrv approved these changes Nov 9, 2016

View reviewed changes

raghavrv changed the title ~~[MRG+1] DOC adding :user: role to whats_new~~ [MRG + 2] DOC adding :user: role to whats_new Nov 9, 2016

raghavrv merged commit c56925a into scikit-learn:master Nov 9, 2016

dalmia deleted the 7748 branch November 16, 2016 13:03


		.. _Scott White: https://twitter.com/scottblanc

		.. _Jaques Grobler: https://github.com/jaquesgrobler/scikit-learn/wiki/Jaques-Grobler


		.. _Dylan Werner-Meier: https://github.com/unautre
		.. _Nelle Varoquaux: https://github.com/nellev

Uh oh!

Conversation

dalmia commented Nov 3, 2016

Reference Issue

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

maniteja123 commented Nov 3, 2016

Uh oh!

dalmia commented Nov 3, 2016

Uh oh!

maniteja123 commented Nov 3, 2016

Uh oh!

jnothman commented Nov 3, 2016

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

jnothman commented Nov 4, 2016 • edited by TomDLT Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

jnothman commented Nov 4, 2016

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

TomDLT commented Nov 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

dalmia commented Nov 4, 2016

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Nov 5, 2016

Uh oh!

dalmia commented Nov 5, 2016

Uh oh!

jnothman commented Nov 6, 2016

Uh oh!

dalmia commented Nov 7, 2016

Uh oh!

raghavrv commented Nov 7, 2016

Uh oh!

jnothman commented Nov 7, 2016

jnothman commented Nov 4, 2016 •

edited by TomDLT

Loading

TomDLT commented Nov 4, 2016 •

edited

Loading