Skip to content

Passing strings to variable_types - #988

Merged
tuethan1999 merged 17 commits into
masterfrom
iss946
May 27, 2020
Merged

Passing strings to variable_types#988
tuethan1999 merged 17 commits into
masterfrom
iss946

Conversation

@tuethan1999

@tuethan1999 tuethan1999 commented May 21, 2020

Copy link
Copy Markdown
Contributor

Pull Request Description

Allows the passing of strings into the variable types dict.

  • Old format: dict[str -> Variable]
  • New format: dict[str ->Variable/str]

After creating the pull request: in order to pass the changelog_updated check you will need to update the "Future Release" section of docs/source/changelog.rst to include this pull request.

@tuethan1999 tuethan1999 linked an issue May 21, 2020 that may be closed by this pull request
@codecov

codecov Bot commented May 21, 2020

Copy link
Copy Markdown

Codecov Report

Merging #988 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #988      +/-   ##
==========================================
+ Coverage   98.25%   98.27%   +0.01%     
==========================================
  Files         119      119              
  Lines       11030    11076      +46     
==========================================
+ Hits        10838    10885      +47     
+ Misses        192      191       -1     
Impacted Files Coverage Δ
featuretools/entityset/entityset.py 96.75% <ø> (+0.27%) ⬆️
featuretools/entityset/deserialize.py 100.00% <100.00%> (ø)
featuretools/entityset/entity.py 95.94% <100.00%> (+0.15%) ⬆️
featuretools/tests/entityset_tests/test_entity.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 d502e75...2b7e432. Read the comment docs.


entity = es["reversed_variable_types"]
for variable in entity.variables:
description = variable.to_data_description()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can test the variable types were set correctly more directly by checking the class of each variable in the entity without serializing the variables

tuethan1999 added 2 commits May 26, 2020 11:41
… the variable types directly. Added a testing for passing strings as variable types within the dfs function
Comment thread featuretools/entityset/entity.py
Comment thread featuretools/entityset/entity.py Outdated
variable_types[vid] = string_to_class_map[vtype]
else:
variable_types[vid] = string_to_class_map['unknown']
warnings.warn("Variable type {} was unrecognized, Unknown Class was used instead".format(vtype))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instead of "Unknown Class" let's say "Unknown variable type"

Comment thread featuretools/tests/entityset_tests/test_entity.py
entity = es["reversed_variable_types"]
for variable in entity.variables:
variable_class = variable.__class__
assert variable_class == variable_types[variable_class.type_string]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should compare the variable_class to the matching variable in the reversed_variable_types dictionary since that was the dictionary used for variable_types

for variable in entity.variables:
description = variable.to_data_description()
_variable = deserialize.description_to_variable(description, entity=entity)
assert variable.__eq__(_variable)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this test should use the same approach as test_passing_strings_to_variable_types_from_dataframe

entities = {'teams': (teams, 'id', None, {'name': 'text'}), 'games': (games, 'id')}
relationships = [('teams', 'id', 'games', 'home_team_id')]

_, features = ft.dfs(entities, relationships, target_entity="teams")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

adding the optional parameter features_only=True will make dfs only return the features list

relationships = [('teams', 'id', 'games', 'home_team_id')]

_, features = ft.dfs(entities, relationships, target_entity="teams")
name_class = features[0].entity.variables[1].__class__

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

An entity's variables can be looked up by name directly from the entity object

features[0].entity['name'].__class__

Small change but I think it makes the test a little easier to follow

@tuethan1999 tuethan1999 changed the title added support for passing strings within variable_types for the entit… Passing strings to variable_types May 27, 2020

@rwedge rwedge left a comment

Copy link
Copy Markdown
Contributor

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 3170cac into master May 27, 2020
@rwedge rwedge mentioned this pull request May 29, 2020
@rwedge
rwedge deleted the iss946 branch June 17, 2020 18:51
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.

Support passing strings to variable_types in DFS

2 participants