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

Replaced type string with camel to snake-case function #996

Merged
merged 9 commits into from
May 28, 2020

Conversation

tuethan1999
Copy link
Contributor

@tuethan1999 tuethan1999 commented May 26, 2020

Pull Request Description

Replaced the Variable class and subclass'stype_string with a function that does a camel to snake case conversion based on the class name. Updated the schema version to reflect this as the LatLong and ZIPcode variable types were affected

tuethan1999 added 2 commits May 26, 2020 14:11
…onversion. Updated the schema version to reflect this as the LatLong and ZIPcode variable types were affected
@codecov
Copy link

codecov bot commented May 26, 2020

Codecov Report

Merging #996 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #996      +/-   ##
==========================================
- Coverage   98.27%   98.27%   -0.01%     
==========================================
  Files         119      119              
  Lines       11076    11059      -17     
==========================================
- Hits        10885    10868      -17     
  Misses        191      191              
Impacted Files Coverage Δ
featuretools/entityset/serialize.py 100.00% <100.00%> (ø)
featuretools/feature_base/features_serializer.py 100.00% <100.00%> (ø)
...etools/tests/entityset_tests/test_serialization.py 99.54% <100.00%> (ø)
...ests/primitive_tests/test_feature_serialization.py 99.31% <100.00%> (ø)
.../tests/primitive_tests/test_features_serializer.py 100.00% <100.00%> (ø)
featuretools/utils/gen_utils.py 100.00% <100.00%> (ø)
featuretools/variable_types/variable.py 98.71% <100.00%> (-0.16%) ⬇️

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 3170cac...c9840d9. Read the comment docs.

@tuethan1999 tuethan1999 changed the title replaced type_string with a function that does a camel to snakecase c… Replaced type string with camel to snake-case function May 27, 2020
@tuethan1999 tuethan1999 requested a review from rwedge May 27, 2020 16:08
@@ -11,7 +11,7 @@ Changelog
* Fixes
* Fix errors with Equals and NotEquals primitives when comparing categoricals or different dtypes (:pr:`968`)
* Normalized type_strings of ``Variable`` classes so that the ``find_variable_types`` function produces a
dictionary with a clear key to name transition (:pr:`982`)
dictionary with a clear key to name transition (:pr:`982`)(:pr:`996`)
Copy link
Contributor

Choose a reason for hiding this comment

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

use (:pr:`982`, :pr:`996`)

featuretools/variable_types/variable.py Show resolved Hide resolved


class ClassNameDescriptor(object):
"""Descriptor to derive the a variable's type_string from it's class name
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the description could be a little more general, we might use this descriptor for other classes besides Variable and it's subclasses

@@ -5,7 +5,7 @@


class ClassNameDescriptor(object):
"""Descriptor to derive the a variable's type_string from it's class name
"""Descriptor to convert a class's name from camelcase to snakcase
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in snakecase word

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

@tuethan1999 tuethan1999 merged commit ec7bcff into master May 28, 2020
@rwedge rwedge mentioned this pull request May 29, 2020
@gsheni gsheni deleted the vtype_function branch June 1, 2020 18:28
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.

2 participants