Skip to content

Fix bug with row count comparison for expect_columns_equivalent(). - #11059

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-22.08from
nvdbaranec:column_utils_row_count
Jun 15, 2022
Merged

Fix bug with row count comparison for expect_columns_equivalent().#11059
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-22.08from
nvdbaranec:column_utils_row_count

Conversation

@nvdbaranec

Copy link
Copy Markdown
Contributor

Top level row counts were being compared incorrectly for the _equivalent() check. No tests were affected by this, thankfully. In addition, even though _equal() was correct, I made the same modification to the call (passing separate index lists for the two columns) to future-proof against internal changes to the comparison function.

@nvdbaranec nvdbaranec added bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Jun 6, 2022
@nvdbaranec
nvdbaranec requested a review from a team as a code owner June 6, 2022 19:37

@ttnghia ttnghia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you can add const after auto then it's perfect to me.

Comment thread cpp/tests/utilities/column_utilities.cu
@vyasr

vyasr commented Jun 7, 2022

Copy link
Copy Markdown
Contributor

rerun tests

@nvdbaranec nvdbaranec added the DO NOT MERGE Hold off on merging; see PR for details label Jun 7, 2022
@nvdbaranec

Copy link
Copy Markdown
Contributor Author

I'm out until Monday, so let's hold off on merging this until then :)

@nvdbaranec nvdbaranec removed the DO NOT MERGE Hold off on merging; see PR for details label Jun 13, 2022
@nvdbaranec

Copy link
Copy Markdown
Contributor Author

rerun tests

1 similar comment
@nvdbaranec

Copy link
Copy Markdown
Contributor Author

rerun tests

@codecov

codecov Bot commented Jun 15, 2022

Copy link
Copy Markdown

Codecov Report

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

@@               Coverage Diff               @@
##             branch-22.08   #11059   +/-   ##
===============================================
  Coverage                ?   86.34%           
===============================================
  Files                   ?      144           
  Lines                   ?    22738           
  Branches                ?        0           
===============================================
  Hits                    ?    19632           
  Misses                  ?     3106           
  Partials                ?        0           

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 10c4c33...e080fcd. Read the comment docs.

@nvdbaranec

Copy link
Copy Markdown
Contributor Author

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit fff29be into NVIDIA:branch-22.08 Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants