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

Include DateTime columns in mutual info calculation #399

Merged
merged 4 commits into from Nov 18, 2020
Merged

Include DateTime columns in mutual info calculation #399

merged 4 commits into from Nov 18, 2020

Conversation

tamargrey
Copy link
Contributor

@codecov
Copy link

codecov bot commented Nov 17, 2020 •
edited

Codecov Report

Merging #399 (9a4b8ca) into main (0a351d6) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #399   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           31        31           
  Lines         3813      3818    +5     
=========================================
+ Hits          3813      3818    +5
Impacted Files Coverage Δ
woodwork/tests/conftest.py 100.00% <ø> (ø)
woodwork/datatable.py 100.00% <100.00%> (ø)
woodwork/tests/datatable/test_datatable.py 100.00% <100.00%> (ø)

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 0a351d6...9a4b8ca. Read the comment docs.

thehomebrewnerd
thehomebrewnerd previously requested changes Nov 17, 2020
View reviewed changes
Copy link
Contributor

@thehomebrewnerd thehomebrewnerd left a comment

Choose a reason for hiding this comment

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

Looks good overall. We also need to update the docstring for mutual_info now that Datetimes are valid and also the Gain Statistical Insights into Your DataTable guide as that mentions that Datetimes are not valid.

docs/source/release_notes.rst Outdated Show resolved Hide resolved
woodwork/datatable.py Show resolved Hide resolved
gsheni
gsheni approved these changes Nov 17, 2020
View reviewed changes
Copy link
Contributor

@gsheni gsheni left a comment

Choose a reason for hiding this comment

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

looks good

woodwork/datatable.py Show resolved Hide resolved
woodwork/datatable.py Show resolved Hide resolved
@gsheni gsheni dismissed thehomebrewnerd’s stale review November 17, 2020 23:29

issues were resolved.

@tamargrey tamargrey merged commit 0545636 into main Nov 18, 2020
@tamargrey tamargrey deleted the mi-datetime branch November 18, 2020 14:30
@gsheni gsheni mentioned this pull request Nov 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thehomebrewnerd thehomebrewnerd thehomebrewnerd left review comments

@gsheni gsheni gsheni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Datetime columns in Mutual Info calculation
3 participants
@tamargrey @thehomebrewnerd @gsheni