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.

Sign up for GitHub

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

Jump to bottom

Remove cardinality computation #438

Merged
merged 4 commits into from
Nov 15, 2019
Merged

Remove cardinality computation #438

merged 4 commits into from
Nov 15, 2019

Conversation

TuanNguyen27
Copy link
Collaborator

@TuanNguyen27 TuanNguyen27 commented Nov 15, 2019 •
edited
Loading

This will be replaced by a more accurate (albeit approximate) frequency count, such as CountMinSketch. cc @Jauntbox

@codecov
Copy link

codecov bot commented Nov 15, 2019 •
edited
Loading

Codecov Report

Merging #438 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
- Coverage   86.93%   86.92%   -0.02%     
==========================================
  Files         337      337              
  Lines       11100    11096       -4     
  Branches      366      362       -4     
==========================================
- Hits         9650     9645       -5     
- Misses       1450     1451       +1
Impacted Files Coverage Δ
...om/salesforce/op/filters/FeatureDistribution.scala 98.55% <100%> (-0.08%) ⬇️
.../op/features/types/FeatureTypeSparkConverter.scala 98.23% <0%> (-0.89%) ⬇️

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 ccc1501...1479746. Read the comment docs.

Jauntbox
Jauntbox approved these changes Nov 15, 2019
View reviewed changes
Copy link
Contributor

@Jauntbox Jauntbox left a comment

Choose a reason for hiding this comment

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

LGTM

@TuanNguyen27 TuanNguyen27 merged commit 52585b6 into master Nov 15, 2019
@tovbinm tovbinm deleted the removeCard branch November 26, 2019 05:10
winterslu added a commit that referenced this pull request Dec 11, 2019
@winterslu
Revert "Remove cardinality computation (#438)"
84002db 
This reverts commit 52585b6.
@winterslu winterslu mentioned this pull request Dec 11, 2019
Merged
@winterslu winterslu mentioned this pull request May 4, 2020
Merged
@nicodv nicodv mentioned this pull request Jun 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jauntbox Jauntbox Jauntbox approved these changes

@gerashegalov gerashegalov Awaiting requested review from gerashegalov

@leahmcguire leahmcguire Awaiting requested review from leahmcguire leahmcguire is a code owner

@tovbinm tovbinm Awaiting requested review from tovbinm tovbinm is a code owner

@wsuchy wsuchy Awaiting requested review from wsuchy

Assignees
No one assigned
Labels
cla:signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@TuanNguyen27 @Jauntbox