Skip to content

[WIP] DOC Ensures that FeatureAgglomeration passes numpydoc validation #20851

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

Conversation

baam25simo
Copy link
Contributor

Reference Issues/PRs

Addresses #20308

What does this implement/fix? Explain your changes.

  • FeatureAgglomeration removed from DOCSTRING_IGNORE_LIST in test_docstrings.py
  • In FeatureAgglomeration : "See Also" section added.
  • In FeatureAgglomeration.fit : **params parameter description added, self description added.
  • In AgglomerationTransform.inverse_transform : summary modified.
  • In AgglomerationTransform.transform : summary modified.

Any other comments?

Running pytest maint_tools/test_docstrings.py -k FeatureAgglomeration- in this branch I obtain:

====================================== test session starts ======================================
platform linux -- Python 3.8.5, pytest-6.2.4, py-1.10.0, pluggy-0.13.1
rootdir: /.../.../scikit-learn, configfile: setup.cfg
plugins: cov-2.12.1, anyio-2.2.0
collected 1760 items / 1752 deselected / 8 selected                                             

maint_tools/test_docstrings.py ..F.....                                                   [100%]

=========================================== FAILURES ============================================
_______________________ test_docstring[FeatureAgglomeration-fit_predict] ________________________

Estimator = <class 'sklearn.cluster._agglomerative.FeatureAgglomeration'>, method = 'fit_predict'
request = <FixtureRequest for <Function test_docstring[FeatureAgglomeration-fit_predict]>>

    @pytest.mark.parametrize("Estimator, method", get_all_methods())
    def test_docstring(Estimator, method, request):
        base_import_path = Estimator.__module__
        import_path = [base_import_path, Estimator.__name__]
        if method is not None:
            import_path.append(method)
    
        import_path = ".".join(import_path)
    
        if Estimator.__name__ in DOCSTRING_IGNORE_LIST:
            request.applymarker(
                pytest.mark.xfail(run=False, reason="TODO pass numpydoc validation")
            )
    
        res = numpydoc_validation.validate(import_path)
    
        res["errors"] = list(filter_errors(res["errors"], method, Estimator=Estimator))
    
        if res["errors"]:
            msg = repr_errors(res, Estimator, method)
    
>           raise ValueError(msg)
E           ValueError: 
E           
E           None
E           
E           FeatureAgglomeration.fit_predict
E           Parsing of the method signature failed, possibly because this is a property.
E           
E           Fit the hierarchical clustering from features or distance matrix,
E           and return cluster labels.
E           
E           Parameters
E           ----------
E           X : array-like of shape (n_samples, n_features) or                 (n_samples, n_samples)
E               Training instances to cluster, or distances between instances if
E               ``affinity='precomputed'``.
E           
E           y : Ignored
E               Not used, present here for API consistency by convention.
E           
E           Returns
E           -------
E           labels : ndarray of shape (n_samples,)
E               Cluster labels.
E           
E           # Errors
E           
E            - GL08: The object does not have a docstring

maint_tools/test_docstrings.py:252: ValueError
========================= 1 failed, 7 passed, 1752 deselected in 1.03s ==========================

which I was not able to solve. Any suggestions?

Thank you!

@baam25simo
Copy link
Contributor Author

Found out that
AgglomerativeClustering.fit_predict() returns super().fit_predict(X, y)
which is ClusterMixin.fit_predict() which returns self.labels_
which is actually an instance constant declared in _agglomerative.py lines 976-982

Can numpydoc validation be considered passed?

@glemaitre
Copy link
Member

I am closing this one because we already reviewed the following that was open before: #20544

@glemaitre glemaitre closed this Aug 31, 2021
@baam25simo baam25simo deleted the FeatureAgglomeration-numpydoc-validation branch August 31, 2021 16:04
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.

2 participants