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

Add doctest #4618

Merged
merged 36 commits into from
Mar 29, 2022
Merged

Add doctest #4618

merged 36 commits into from
Mar 29, 2022

Conversation

lowener
Copy link
Contributor

@lowener lowener commented Mar 7, 2022

Continuation of #2975 to change docstring to a doctest format.
I'm also adding a pytest file to execute doctest, similar to what's being done in cudf

@lowener lowener added the doc Documentation label Mar 10, 2022
@lowener lowener requested a review from a team as a code owner March 11, 2022 18:25
@lowener lowener added 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Mar 11, 2022
@lowener
Copy link
Contributor Author

lowener commented Mar 17, 2022

rerun tests

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.

Changes look fantastic, took me a bit of time but just had very few minor requests

python/cuml/cluster/kmeans.pyx Outdated Show resolved Hide resolved
Comment on lines +73 to +84
>>> print(X.compute()) # doctest: +SKIP
[[-1.1273878 1.2844919 -0.32349187 0.1595734 ]
[ 0.80521786 -0.65946865 -0.40753683 0.15538901]
[ 1.0404129 -1.481386 1.4241115 1.2664981 ]
[-0.92821544 -0.6805706 -0.26001272 0.36004275]
[-1.0392245 -1.1977317 0.16345565 -0.21848428]
[ 1.2273135 -0.529214 2.4799604 0.44108105]
[-1.9163864 -0.39505136 -1.9588828 -1.8881643 ]
[-0.9788184 -0.89851004 -0.08339313 0.1130247 ]
[-1.0549078 -0.8993015 -0.11921967 0.04821599]
[-1.8388828 -1.4063598 -0.02838472 -1.0874642 ]]
>>> print(y.compute()) # doctest: +SKIP
Copy link
Member

Choose a reason for hiding this comment

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

Why these skips? Question applies to all instances in the dask docstrings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly because of numerical imprecision. The output for dask are harder to reproduce so they are skipped more often. I added a few lines to the developer guide regarding this.

python/cuml/dask/preprocessing/label.py Show resolved Hide resolved
@lowener lowener requested a review from dantegd March 25, 2022 22:03
@lowener
Copy link
Contributor Author

lowener commented Mar 26, 2022

rerun tests

@dantegd
Copy link
Member

dantegd commented Mar 28, 2022

@gpucibot merge

@dantegd
Copy link
Member

dantegd commented Mar 28, 2022

rerun tests

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.04@3798925). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.04    #4618   +/-   ##
===============================================
  Coverage                ?   83.97%           
===============================================
  Files                   ?      252           
  Lines                   ?    20336           
  Branches                ?        0           
===============================================
  Hits                    ?    17078           
  Misses                  ?     3258           
  Partials                ?        0           
Flag Coverage Δ
dask 44.98% <0.00%> (?)
non-dask 77.26% <0.00%> (?)

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


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 3798925...e0e400f. Read the comment docs.

@dantegd
Copy link
Member

dantegd commented Mar 28, 2022

@lowener I think all the sphinx warnings that appear look like:

/opt/conda/envs/rapids/lib/python3.9/site-packages/numpydoc/docscrape.py:434: UserWarning: potentially wrong underline length... 
Returns 
---------- in 
KMeans.transform(self, X, convert_dtype=False) -> CumlArray

and can be solved by changing this line:

https://github.com/lowener/cuml/blob/e0e400f87c93cf1b4599b6840cdf6c8912352f68/python/cuml/common/doc_utils.py#L322

to:

                '\nReturns\n-------\n'

@rapids-bot rapids-bot bot merged commit 6b72323 into rapidsai:branch-22.04 Mar 29, 2022
@lowener lowener deleted the 22.04-doctest branch March 29, 2022 20:19
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
Continuation of rapidsai#2975 to change docstring to a doctest format.
I'm also adding a pytest file to execute doctest, similar to what's being done in [cudf](https://github.com/rapidsai/cudf/blob/branch-22.04/python/cudf/cudf/tests/test_doctests.py)

Authors:
  - Micka (https://github.com/lowener)
  - Yuqiong Li (https://github.com/yuqli)
  - Dante Gama Dessavre (https://github.com/dantegd)
  - Michael Demoret (https://github.com/mdemoret-nv)

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

URL: rapidsai#4618
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team 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