Skip to content

adding versionadded to a few modules#16015

Closed
adrinjalali wants to merge 53 commits intoscikit-learn:masterfrom
adrinjalali:reshamas/adding_version
Closed

adding versionadded to a few modules#16015
adrinjalali wants to merge 53 commits intoscikit-learn:masterfrom
adrinjalali:reshamas/adding_version

Conversation

@adrinjalali
Copy link
Copy Markdown
Member

Closes #15973

Somehow I couldn't push to the original repo, but had addressed some of the comments.

Circe McDonald and others added 30 commits November 2, 2019 11:37
Copy link
Copy Markdown
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

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

I still might be more comfortable if this was implemented one what's new file at a time (and certainly not with the 2 files max limit proposed on the other PR). and with care to only include newly introduced estimators...

If a callable is passed it is used to extract the sequence of features
out of the raw, unprocessed input.

.. versionchanged:: 0.21
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this requires same formatting as above

Xin Dang, Hanxiang Peng, Xueqin Wang and Heping Zhang
http://home.olemiss.edu/~xdang/papers/MTSE.pdf

.. versionadded:: v0.16
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why the v?


Read more in the :ref:`User Guide <cross_validation>`.

.. versionadded:: 0.13
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

depends how you account for the transition from sklearn.cross_validation to sklearn.model_selection...

with a test set of size ``n_samples//(n_splits + 1)``,
where ``n_samples`` is the number of samples.

..versionadded:: v0.18
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have 0.19 in the summary above... and 0.18 here in the Notes

Information Retrieval. Cambridge University Press, pp. 234-265.
https://nlp.stanford.edu/IR-book/html/htmledition/naive-bayes-text-classification-1.html

.. versionchanged:: v0.16
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can't be in References. and I'm not sure whether we want this level of detail, if it implies maintaining such per-estimator changelogs elsewhere (which would be neat but is a lot of work)

(Vol. 3, pp. 616-623).
https://people.csail.mit.edu/jrennie/papers/icml03-nb.pdf

.. versionadded:: v0.20
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In references, and includes "v"


Read more in the :ref:`User Guide <classification>`.

.. versionadded:: 0.22
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think so!


Read more in the :ref:`User Guide <classification>`.

.. versionadded:: 0.14
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Really, this late?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No, it's incorrect

foo_param
expected_fit_params

.. versionchanged:: 0.22
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not in Parameters... probably can rm altogether

@adrinjalali
Copy link
Copy Markdown
Member Author

I still might be more comfortable if this was implemented one what's new file at a time (and certainly not with the 2 files max limit proposed on the other PR). and with care to only include newly introduced estimators...

@jnothman I'm happy to close this and have one changelog at a time as you propose, which should be fine as a few issues for the sprints. But you also leave a review, which makes me think I should apply them :P should I ignore your review and close this and the other PR and leave it as you suggest for the sprints?

@jnothman
Copy link
Copy Markdown
Member

jnothman commented Jan 8, 2020 via email

@cmarmo
Copy link
Copy Markdown
Contributor

cmarmo commented Jan 9, 2020

@adrinjalali, @jnothman , may I suggest to close this PR and let the exercise of checking the version_added to people at the sprint? @glemaitre has added some recommendations about how to make the PR more reviewable. Feel free to complete if necessary.

@adrinjalali
Copy link
Copy Markdown
Member Author

Yep, closing. Thank you all :)

@adrinjalali adrinjalali closed this Jan 9, 2020
@adrinjalali adrinjalali deleted the reshamas/adding_version branch January 9, 2020 14:24
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