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

Python 3 compatibility #16

Merged
merged 15 commits into from
Dec 20, 2017
Merged

Python 3 compatibility #16

merged 15 commits into from
Dec 20, 2017

Conversation

joshblum
Copy link
Contributor

@joshblum joshblum commented Oct 17, 2017

No description provided.

@joshblum joshblum requested a review from kmax12 October 17, 2017 22:23
MANIFEST.in Outdated
@@ -0,0 +1,4 @@
include requirements.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Required for tox to see the requirement files referenced in setup.py

@@ -53,8 +53,8 @@ def initialize_logging(config):
err_handler.setFormatter(logging.Formatter(fmt))
err_levels = ['WARNING', 'ERROR', 'CRITICAL']

for name, level in loggers.items():
LEVEL = logging._levelNames[level.upper()]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this private variable changed in python3, getattr should have the same functionality and work across python 2.x and 3.x

from collections import Mapping, Set, deque
from numbers import Number

from past.builtins import basestring

from pympler.asizeof import asizeof as getsize
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The getsize method errors on calculating the size on a pandas SingleBlockManager object. The reference in the docstring also suggested the pympler library so I used that instead

@joshblum joshblum force-pushed the py2to3 branch 5 times, most recently from 2d327e5 to a049be0 Compare October 17, 2017 22:56
@@ -27,7 +33,7 @@ def load_mock_customer(n_customers=5, n_products=5, n_sessions=35, n_transaction
transactions_df = transactions_df.sort_values("session_id").reset_index(drop=True)
transactions_df["transaction_time"] = pd.date_range('1/1/2014', periods=n_transactions, freq='65s') # todo make these less regular
transactions_df["product_id"] = pd.Categorical(choice(products_df["product_id"], n_transactions))
transactions_df["amount"] = random.randint(500, 15000, n_transactions) / 100.0
transactions_df["amount"] = old_div(random.randint(500, 15000, n_transactions), 100.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

was the swapping in of old_div automatic? In many of these cases, I think we actually can just use the true divide with python 3 and from __future__ import division to support python 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, this was automatically applied during the conversion. Just mark anything that doesn't need it and i'll switch it over :)

@codecov-io
Copy link

codecov-io commented Oct 19, 2017

Codecov Report

Merging #16 into master will decrease coverage by 0.99%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master      #16    +/-   ##
========================================
- Coverage   88.12%   87.12%    -1%     
========================================
  Files          71       74     +3     
  Lines        6829     6937   +108     
========================================
+ Hits         6018     6044    +26     
- Misses        811      893    +82
Impacted Files Coverage Δ
featuretools/demo/__init__.py
...ests/feature_function_tests/test_primitive_base.py
.../feature_function_tests/test_transform_features.py
featuretools/entityset/serialization.py
featuretools/selection/selection.py
featuretools/core/api.py
featuretools/primitives/direct_feature.py
featuretools/core/__init__.py
featuretools/selection/api.py
featuretools/variable_types/__init__.py
... and 95 more

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 de45be2...58bf3bf. Read the comment docs.

@@ -120,7 +125,7 @@ def get_name(self):
unit = self.readable_unit
if self.readable_unit == "Weeks":
# divide to convert back
return "{} {}".format(self.value / self._convert_to_days["w"], unit)
return "{} {}".format(self.value // self._convert_to_days["w"], unit)
Copy link
Contributor Author

@joshblum joshblum Oct 19, 2017

Choose a reason for hiding this comment

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

note // do we want to cast to int?

Copy link
Contributor

Choose a reason for hiding this comment

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

this can be /

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -41,7 +43,7 @@ def get_categorical_nunique_ratio(df, drop_nonumeric=True):

nunique = df.nunique(axis=0, dropna=True)
total = df[nonnumeric_columns].count(axis=0)
return nunique / total
return nunique // total
Copy link
Contributor Author

@joshblum joshblum Oct 19, 2017

Choose a reason for hiding this comment

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

note // do we want to cast to int?

Copy link
Contributor

Choose a reason for hiding this comment

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

this can be /

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -133,7 +135,7 @@ def select_percent_null(feature_matrix, features, max_null_percent=1.0, keep=Non
keep = keep or []

null_counts = feature_matrix.isnull().sum()
null_percents = null_counts / feature_matrix.shape[0]
null_percents = null_counts // feature_matrix.shape[0]
Copy link
Contributor Author

@joshblum joshblum Oct 19, 2017

Choose a reason for hiding this comment

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

note // do we want to cast to int?

Copy link
Contributor

Choose a reason for hiding this comment

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

this can be /

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@soedr
Copy link

soedr commented Nov 28, 2017

Thanks for working towards making this available to python 3 users! Is there a high-level estimate of when it is expected to be released?

@kmax12
Copy link
Contributor

kmax12 commented Nov 28, 2017

@soedr working on it for the next release. if you sign up for the mailing list here, we will email when it's released.

@kmax12 kmax12 merged commit 99a80d7 into alteryx:master Dec 20, 2017
@kmax12 kmax12 mentioned this pull request Dec 20, 2017
@joshblum joshblum deleted the py2to3 branch December 20, 2017 20:37
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.

None yet

4 participants