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

Adding docs for .get_feature_names() inside TfidfVectorizer #4226

Conversation

mayankanand007
Copy link
Contributor

This PR resolves issue #4219 by adding docs for .get_feature_names() in the TfidfVectorizer class.

As mentioned in the linked issue, the method already exists in CountVectorizer and TfidfVectorizer inherits from that class, hence the functionality is present but not documented.

@mayankanand007 mayankanand007 requested a review from a team as a code owner September 22, 2021 22:33
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Sep 22, 2021
@beckernick beckernick added doc Documentation non-breaking Non-breaking change labels Sep 23, 2021
Copy link
Member

@dantegd dantegd left a comment

Choose a reason for hiding this comment

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

Just one change needed for correct copyright

python/cuml/feature_extraction/_tfidf_vectorizer.py Outdated Show resolved Hide resolved
@dantegd dantegd added the 4 - Waiting on Author Waiting for author to respond to review label Sep 24, 2021
@@ -256,3 +256,13 @@ def transform(self, raw_documents):
"""
X = super().transform(raw_documents)
return self._tfidf.transform(X, copy=False)

def get_feature_names(self):
Copy link
Member

Choose a reason for hiding this comment

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

Just after approving remembered that I forgot to ask, could you add a very simple pytest so that this method is covered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@NV-jpt
Copy link
Contributor

NV-jpt commented Sep 24, 2021

rerun tests

2 similar comments
@mayankanand007
Copy link
Contributor Author

rerun tests

@mayankanand007
Copy link
Contributor Author

rerun tests

@codecov-commenter
Copy link

Codecov Report

Merging #4226 (3a15a2a) into branch-21.10 (a69db68) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.10    #4226      +/-   ##
================================================
+ Coverage         86.07%   86.09%   +0.01%     
================================================
  Files               231      231              
  Lines             18649    18690      +41     
================================================
+ Hits              16052    16091      +39     
- Misses             2597     2599       +2     
Flag Coverage Δ
dask 47.12% <50.00%> (+0.09%) ⬆️
non-dask 78.77% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ython/cuml/feature_extraction/_tfidf_vectorizer.py 88.67% <100.00%> (+0.44%) ⬆️
python/cuml/linear_model/mbsgd_classifier.pyx 100.00% <0.00%> (ø)
python/cuml/linear_model/linear_regression.pyx 92.68% <0.00%> (+0.91%) ⬆️

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 a69db68...3a15a2a. Read the comment docs.

@dantegd
Copy link
Member

dantegd commented Sep 27, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit fb39648 into rapidsai:branch-21.10 Sep 27, 2021
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
…dsai#4226)

This PR resolves issue rapidsai#4219 by adding docs for `.get_feature_names()` in the `TfidfVectorizer` class.

As mentioned in the linked issue, the method already exists in `CountVectorizer` and `TfidfVectorizer` inherits from that class, hence the functionality is present but not documented.

Authors:
  - Mayank Anand (https://github.com/mayankanand007)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: rapidsai#4226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Waiting on Author Waiting for author to respond to review Cython / Python Cython or Python issue doc Documentation non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants