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

WIP: independence tests for categorical variables #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

schnirz
Copy link
Collaborator

@schnirz schnirz commented Dec 9, 2018

implemented permutation-based independence tests for categorical variables based on mutual information and conditional mutual information

open questions:

  • can type information be used more elegantly/efficiently?
  • is the conditional mutual information test implemented correctly? do we need more tests to be sure?
  • are there more efficient entropy estimators that we should implement?

@schnirz schnirz requested a review from mschauer December 9, 2018 13:41
@codecov-io
Copy link

codecov-io commented Dec 9, 2018

Codecov Report

Merging #11 into master will increase coverage by 0.95%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
+ Coverage   83.99%   84.95%   +0.95%     
==========================================
  Files           7        7              
  Lines         606      638      +32     
==========================================
+ Hits          509      542      +33     
+ Misses         97       96       -1
Impacted Files Coverage Δ
src/pc.jl 89.18% <100%> (+0.79%) ⬆️
src/klentropy.jl 89.21% <100%> (+3.31%) ⬆️
src/skeleton.jl 89.55% <100%> (+1.62%) ⬆️

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 f354ca8...b79fa8e. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.0%) to 84.953% when pulling b79fa8e on categorical_tests into f354ca8 on master.

@mschauer
Copy link
Owner

mschauer commented Apr 8, 2019

Do you think you can make this work without my input? Please merge once you think this is fine.

@schnirz
Copy link
Collaborator Author

schnirz commented Apr 21, 2019

Sure, no problem

Copy link
Owner

@mschauer mschauer left a comment

Choose a reason for hiding this comment

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

If you feel confident please merge

@mschauer
Copy link
Owner

@schnirz Is this "okay enough" for me to just merge?

@schnirz
Copy link
Collaborator Author

schnirz commented Oct 7, 2019

@mschauer I finally have some time this month to do more work on this, will let you know once I have some sensible ready.

@mschauer
Copy link
Owner

mschauer commented Oct 7, 2019

This is great news. There has been also some interest for this package in the last weeks, so this would be a good time.

@mschauer
Copy link
Owner

Yearly reminder ;-)

@mschauer mschauer mentioned this pull request Oct 10, 2020
c = Tables.columns(t)
sch = Tables.schema(t)
n = length(sch.names)

return pcalg(n, cmitest, c, p; kwargs...)
return pcalg(n, cmitest, c, sch, p; kwargs...)
Copy link
Owner

Choose a reason for hiding this comment

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

Is that defined?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants