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

Equal and NotEqual fix #968

Merged
merged 12 commits into from
May 21, 2020
Merged

Equal and NotEqual fix #968

merged 12 commits into from
May 21, 2020

Conversation

frances-h
Copy link
Contributor

@frances-h frances-h commented May 14, 2020 •
edited
Loading

Resolves #922 and #944

  • Force categories of categoricals to match
  • Use native .eq instead of numpy.equal

@codecov
Copy link

codecov bot commented May 14, 2020 •
edited
Loading

Codecov Report

Merging #968 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #968   +/-   ##
=======================================
  Coverage   98.23%   98.24%           
=======================================
  Files         119      119           
  Lines       10905    10945   +40     
=======================================
+ Hits        10713    10753   +40     
  Misses        192      192
Impacted Files Coverage Δ
...aturetools/primitives/standard/binary_transform.py 100.00% <100.00%> (ø)
...s/tests/primitive_tests/test_transform_features.py 98.56% <100.00%> (+0.07%) ⬆️

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 d8843c0...f2c36b1. Read the comment docs.

@rwedge rwedge linked an issue May 18, 2020 that may be closed by this pull request
Equal, NotEqual primitives error when comparing date, object columns bug #944
Closed
@frances-h frances-h force-pushed the equal-not-equal-fix branch from 4f1462b to 495e5b5 Compare May 20, 2020 13:18
rwedge
rwedge reviewed May 21, 2020
View reviewed changes
featuretools/tests/primitive_tests/test_transform_features.py Outdated
})

es = ft.EntitySet('not_equal_test')
es.entity_from_dataframe('values', df, index='id')
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this entityset a fixture use by both tests to avoid duplicated code?

rwedge
rwedge reviewed May 21, 2020
View reviewed changes
Copy link
Contributor

@rwedge rwedge left a comment

Choose a reason for hiding this comment

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

Can we also add a test case for the issue reported in #944

rwedge
rwedge approved these changes May 21, 2020
View reviewed changes
Copy link
Contributor

@rwedge rwedge 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

@frances-h frances-h merged commit 4f270d5 into master May 21, 2020
@frances-h frances-h deleted the equal-not-equal-fix branch May 21, 2020 21:25
@rwedge rwedge mentioned this pull request May 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rwedge rwedge rwedge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@frances-h @rwedge