Skip to content

Support Pandas 0.23.0 #153

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

Merged
merged 14 commits into from
May 22, 2018
Merged

Support Pandas 0.23.0 #153

merged 14 commits into from
May 22, 2018

Conversation

bschreck
Copy link
Contributor

Various changes to support Pandas 0.23. Issues related to the new ability to merge on a combination of index and column names, the need to supply a "sort" argument to DataFrame.append, as well as some issues related to MultiIndex sorting.

cutoff_time['time'] = pd.to_numeric(cutoff_time['time'])
except (ValueError, TypeError):
raise TypeError("cutoff_time times must be a numeric")
else:
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 do an explicit check for DateTimeIndex here?

pass_columns = [column_name for column_name in cutoff_time.columns[2:]]

if _check_time_type(cutoff_time['time'].iloc[0]) is None:
raise ValueError("cutoff_time time values must be datetime or numeric")
if cutoff_time['instance_id'].dtype == np.dtype('O') and target_entity.df[target_entity.index].dtype.name.find('int') > -1:
Copy link
Contributor

@kmax12 kmax12 May 22, 2018

Choose a reason for hiding this comment

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

I think we can do without these 2 checks and conversations for now. let's assume the user has this fixed up before calling calculate feature matrix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are to suppress warnings from the tests. Should we change the tests then? Or add in a filter to ignore pandas warnings when running the tests?

@kmax12 kmax12 changed the title Support Pandas 0.23 Support Pandas 0.23.0 May 22, 2018
@codecov-io
Copy link

codecov-io commented May 22, 2018

Codecov Report

Merging #153 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
- Coverage    92.7%   92.61%   -0.09%     
==========================================
  Files          72       72              
  Lines        7714     7718       +4     
==========================================
- Hits         7151     7148       -3     
- Misses        563      570       +7
Impacted Files Coverage Δ
...ools/tests/entityset_tests/test_last_time_index.py 100% <100%> (ø) ⬆️
...computational_backends/calculate_feature_matrix.py 97.86% <100%> (-0.34%) ⬇️
featuretools/entityset/entity.py 90.8% <100%> (+0.02%) ⬆️
featuretools/entityset/entityset.py 91.4% <100%> (-0.02%) ⬇️
...utational_backend/test_calculate_feature_matrix.py 99.55% <100%> (ø) ⬆️
...turetools/computational_backends/pandas_backend.py 92.1% <100%> (+0.02%) ⬆️
featuretools/primitives/binary_transform.py 87.28% <0%> (-3.47%) ⬇️

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 14cedde...7ae465b. Read the comment docs.

@kmax12
Copy link
Contributor

kmax12 commented May 22, 2018

Looks good to me. Merge it!

@bschreck bschreck merged commit 18f3045 into master May 22, 2018
@bschreck bschreck deleted the pandas-23-fixes branch May 22, 2018 21:12
@rwedge rwedge mentioned this pull request May 30, 2018
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.

3 participants