Skip to content

[MRG] Add docs for TfidfTransformer.idf_ (Fixes #8267) #8528

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

Closed
wants to merge 2 commits into from
Closed

[MRG] Add docs for TfidfTransformer.idf_ (Fixes #8267) #8528

wants to merge 2 commits into from

Conversation

ekellbuch
Copy link

Reference Issue

Fixes #8267

What does this implement/fix? Explain your changes.

Any other comments?

@codecov
Copy link

codecov bot commented Mar 4, 2017

Codecov Report

Merging #8528 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #8528   +/-   ##
=======================================
  Coverage   95.48%   95.48%           
=======================================
  Files         342      342           
  Lines       60913    60913           
=======================================
  Hits        58160    58160           
  Misses       2753     2753
Impacted Files Coverage Δ
sklearn/feature_extraction/text.py 96.05% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fad5f9b...864993a. Read the comment docs.

@@ -189,7 +189,7 @@ def build_preprocessor(self):
# hundreds of nanoseconds which is negligible when compared to the
# cost of tokenizing a string of 1000 chars for instance.
noop = lambda x: x

Copy link
Member

Choose a reason for hiding this comment

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

please remove the whitespace

Attributes
----------
idf_ : numpy array of shape [n_features, 1]
returns None unless use_idf=True, then
Copy link
Member

Choose a reason for hiding this comment

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

We would not usually describe an attribute with "returns".

----------
idf_ : numpy array of shape [n_features, 1]
returns None unless use_idf=True, then
returns 1-D matrix containing idf(d,t).
Copy link
Member

Choose a reason for hiding this comment

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

I don't get what d is and why this 1-d vector is a function of two indices.

Copy link
Member

@raghavrv raghavrv left a comment

Choose a reason for hiding this comment

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

I'm closing the other PR (#8532) as it is a duplicate and this one has more reviews...

@@ -996,6 +996,12 @@ class TfidfTransformer(BaseEstimator, TransformerMixin):
sublinear_tf : boolean, default=False
Apply sublinear tf scaling, i.e. replace tf with 1 + log(tf).

Attributes
----------
idf_ : numpy array of shape [n_features, 1]
Copy link
Member

Choose a reason for hiding this comment

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

Copying @jmschrei's comment from #8532 -

This class doesn't seem to have or use a self.idf_ attribute. Do you mean self._idf_diag?

Copy link
Member

Choose a reason for hiding this comment

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

There is a property.

@raghavrv raghavrv changed the title TfidfTransformer.idf_ doc #8267 [MRG] Add docs for TfidfTransformer.idf_ (Fixes #8267) Mar 7, 2017
@amueller
Copy link
Member

fixed in #10899

@amueller amueller closed this May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TfidfTransformer.idf_ does not appear to be documented
4 participants