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

[REVIEW] adding Corr() function inside dataframe.py to compute the correlation matrix of a data frame and adding examples in series.py #4140

Merged
merged 24 commits into from
Mar 11, 2020

Conversation

rnyak
Copy link
Contributor

@rnyak rnyak commented Feb 12, 2020

No description provided.

@rnyak rnyak requested a review from a team as a code owner February 12, 2020 22:03
@kkraus14 kkraus14 added 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer Python Affects Python cuDF API. labels Feb 13, 2020
python/cudf/cudf/tests/test_stats.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_stats.py Outdated Show resolved Hide resolved
python/cudf/cudf/tests/test_stats.py Outdated Show resolved Hide resolved
@kkraus14 kkraus14 added 0 - Waiting on Author Waiting for author to respond to review and removed 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer labels Feb 13, 2020
@rnyak rnyak changed the title [REVIEW] adding Corr() function inside dataframe.py to compute the correlation matrix of a data frame [REVIEW] adding Corr() function inside dataframe.py to compute the correlation matrix of a data frame and adding examples in series.py Feb 13, 2020
@codecov
Copy link

codecov bot commented Feb 13, 2020

Codecov Report

Merging #4140 into branch-0.13 will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff               @@
##           branch-0.13   #4140      +/-   ##
==============================================
+ Coverage         88.8%   88.8%   +<.01%     
==============================================
  Files               50      50              
  Lines             9691    9696       +5     
==============================================
+ Hits              8606    8611       +5     
  Misses            1085    1085
Impacted Files Coverage Δ
python/cudf/cudf/utils/ioutils.py 86.3% <ø> (ø) ⬆️
python/cudf/cudf/core/series.py 91.65% <ø> (ø) ⬆️
python/cudf/cudf/core/dataframe.py 90.2% <100%> (+0.03%) ⬆️

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 c0a9344...5844786. Read the comment docs.

@rnyak
Copy link
Contributor Author

rnyak commented Feb 19, 2020

@kkraus14 I have done the changes you requested for the unit tests. I also have just added an example in the ioutils.py for reading hex values as int. FYI.

Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

One docstring issue then this looks good to go

CHANGELOG.md Outdated Show resolved Hide resolved
python/cudf/cudf/utils/ioutils.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

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

LGTM, could probably use a review from @randerzander as I always fail at docstring formatting

@kkraus14 kkraus14 added 3 - Ready for Review Ready for review by team doc Documentation and removed 0 - Waiting on Author Waiting for author to respond to review labels Feb 24, 2020
Copy link
Contributor

@randerzander randerzander left a comment

Choose a reason for hiding this comment

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

A small suggestion on hex to integer conversion in the read_csv docstring.

With that change and a confirmation from @rnyak that the Sphinx generated HTML files render correctly, this is good to go. Thanks for helping us improve our API docs!

python/cudf/cudf/utils/ioutils.py Outdated Show resolved Hide resolved
@randerzander
Copy link
Contributor

rerun tests

@kkraus14
Copy link
Collaborator


+ python -c 'import cudf'

Traceback (most recent call last):

  File "<string>", line 1, in <module>

  File "/conda/conda-bld/cudf_1583851712040/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.6/site-packages/cudf/__init__.py", line 4, in <module>

    from cudf import core, datasets

  File "/conda/conda-bld/cudf_1583851712040/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.6/site-packages/cudf/core/__init__.py", line 2, in <module>

    from cudf.core import buffer, column

  File "/conda/conda-bld/cudf_1583851712040/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.6/site-packages/cudf/core/column/__init__.py", line 1, in <module>

    from cudf.core.column.categorical import CategoricalColumn  # noqa: F401

  File "/conda/conda-bld/cudf_1583851712040/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.6/site-packages/cudf/core/column/categorical.py", line 10, in <module>

    import cudf._lib as libcudf

  File "/conda/conda-bld/cudf_1583851712040/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.6/site-packages/cudf/_lib/__init__.py", line 1, in <module>

    from . import (

  File "cudf/_lib/avro.pyx", line 18, in init cudf._lib.avro

  File "/conda/conda-bld/cudf_1583851712040/_test_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.6/site-packages/cudf/utils/ioutils.py", line 801, in <module>

    remote_data_sources=_docstring_remote_sources

KeyError: '"hex_col" '

@rnyak
Copy link
Contributor Author

rnyak commented Mar 11, 2020

@kkraus14 @randerzander will take a look at that what's going on. If we want to merge other docs edits asap, I can take out 'hex_col` lines and then add again and create a new PR once I figure out why we get this error.

@kkraus14
Copy link
Collaborator

@kkraus14 @randerzander will take a look at that what's going on. If we want to merge other docs edits asap, I can take out 'hex_col` lines and then add again and create a new PR once I figure out why we get this error.

Please do.

@kkraus14 kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Mar 11, 2020
@kkraus14
Copy link
Collaborator

rerun tests

@kkraus14 kkraus14 merged commit 8de02da into rapidsai:branch-0.13 Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge doc Documentation Python Affects Python cuDF API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants