Skip to content

Change name of date_of_birth (column name) to birthday in mock dataset#1754

Merged
rwedge merged 9 commits into
alteryx:mainfrom
ridicolos:date_of_birth_to_birthday
Nov 1, 2021
Merged

Change name of date_of_birth (column name) to birthday in mock dataset#1754
rwedge merged 9 commits into
alteryx:mainfrom
ridicolos:date_of_birth_to_birthday

Conversation

@ridicolos

@ridicolos ridicolos commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

@CLAassistant

CLAassistant commented Oct 25, 2021

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@gsheni

gsheni commented Oct 26, 2021

Copy link
Copy Markdown
Contributor

Thanks for the contribution @ridicolos!

@ridicolos

ridicolos commented Oct 26, 2021

Copy link
Copy Markdown
Contributor Author

@gsheni could you maybe explain why so much of these test just failed with the changes I did? I am a little bit confused on what happens here?

What I did find, which could explian some of the errors, is that the tests use csv-files from s3 where the column-name is still set to date_of_birth. You can check it out here: https://featuretools-static.s3.amazonaws.com/test_ecommerce/customers.csv

Do I even have an ability to change this?

@thehomebrewnerd

Copy link
Copy Markdown
Contributor

Do I even have an ability to change this?

@ridicolos I can help you with updating the S3 files when the time comes. In the meantime, there are a couple additional updates that we need to make first. I will comment on the code in the appropriate places.

Namely, we do not want to change any references to the Woodwork semantic tag of date_of_birth.

We will look into the docs build failures separately, as that seems unrelated to your changes.

@thehomebrewnerd thehomebrewnerd 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.

Overall this is looking good. There are a few places where we should leave date_of_birth in place, and I have noted those.

"metadata": {},
"source": [
"Above we see the semantic tags that are defined within Woodwork. These tags inform how Featuretools is able to interpret data, an example of which can be seen in the `Age` primitive, which requires that the `date_of_birth` semantic tag be present on a column.\n",
"Above we see the semantic tags that are defined within Woodwork. These tags inform how Featuretools is able to interpret data, an example of which can be seen in the `Age` primitive, which requires that the `birthday` semantic tag be present on a column.\n",

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.

Do not change here, we should leave the Woodwork tag as date_of_birth.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 315dac2

"Above we see the semantic tags that are defined within Woodwork. These tags inform how Featuretools is able to interpret data, an example of which can be seen in the `Age` primitive, which requires that the `birthday` semantic tag be present on a column.\n",
"\n",
"The `date_of_birth` tag will not get automatically added by Woodwork, so in order for Featuretools to be able to use the `Age` primitive, the `date_of_birth` tag must be manually added to any columns to which it applies.\n",
"The `birthday` tag will not get automatically added by Woodwork, so in order for Featuretools to be able to use the `Age` primitive, the `birthday` tag must be manually added to any columns to which it applies.\n",

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.

Do not change here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 802df3d

"```\n",
"\n",
"Woodwork does not have a specific `DateOfBirth` logical type, but rather identifies a column as a date of birth column by specifying the logical type as `Datetime` with a semantic tag of `date_of_birth`. There is also no `Numeric` logical type in Woodwork, but rather Woodwork identifies all columns that can be used for numeric operations with the semantic tag of `numeric`. Furthermore, we know the `Age` primitive will return a floating point number, which would correspond to a Woodwork logical type of `Double`. With these items in mind, we can redefine the `Age` input types and return types with `ColumnSchema` objects as follows:\n",
"Woodwork does not have a specific `DateOfBirth` logical type, but rather identifies a column as a date of birth column by specifying the logical type as `Datetime` with a semantic tag of `birthday`. There is also no `Numeric` logical type in Woodwork, but rather Woodwork identifies all columns that can be used for numeric operations with the semantic tag of `numeric`. Furthermore, we know the `Age` primitive will return a floating point number, which would correspond to a Woodwork logical type of `Double`. With these items in mind, we can redefine the `Age` input types and return types with `ColumnSchema` objects as follows:\n",

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.

Do not change here.

@ridicolos ridicolos Oct 27, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 315dac2

"\n",
"```python\n",
"input_types = [ColumnSchema(logical_type=Datetime, semantic_tags={'date_of_birth'})]\n",
"input_types = [ColumnSchema(logical_type=Datetime, semantic_tags={'birthday'})]\n",

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.

Do not change here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed in 315dac2

"| CountryCode | ColumnSchema(logical_type=CountryCode) |\n",
"| Datetime | ColumnSchema(logical_type=Datetime) |\n",
"| DateOfBirth | ColumnSchema(logical_type=Datetime, semantic_tags={'date_of_birth'}) |\n",
"| DateOfBirth | ColumnSchema(logical_type=Datetime, semantic_tags={'birthday'}) |\n",

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.

Do not change here.

@ridicolos ridicolos Oct 27, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 315dac2

"""
name = "age"
input_types = [ColumnSchema(logical_type=Datetime, semantic_tags={'date_of_birth'})]
input_types = [ColumnSchema(logical_type=Datetime, semantic_tags={'birthday'})]

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.

Do not change here.

@ridicolos ridicolos Oct 27, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 315dac2


customer_semantic_tags = {u'région_id': 'foreign_key',
'date_of_birth': 'date_of_birth'}
'birthday': 'birthday'}

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 should be 'birthday': 'date_of_birth' since we just want to rename the dataframe column and not the Woodwork tag.

@ridicolos ridicolos Oct 27, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 315dac2

@thehomebrewnerd thehomebrewnerd 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.

There is one more reference to birthday that should be changed back to date_of_birth.

"Above we see the semantic tags that are defined within Woodwork. These tags inform how Featuretools is able to interpret data, an example of which can be seen in the `Age` primitive, which requires that the `date_of_birth` semantic tag be present on a column.\n",
"\n",
"The `birthday` tag will not get automatically added by Woodwork, so in order for Featuretools to be able to use the `Age` primitive, the `birthday` tag must be manually added to any columns to which it applies.\n",
"The `date_of_birth` tag will not get automatically added by Woodwork, so in order for Featuretools to be able to use the `Age` primitive, the `birthday` tag must be manually added to any columns to which it applies.\n",

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.

There is another reference to the birthday tag in this sentence that needs to be changed to date_of_birth.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed with 5a38bc8

@thehomebrewnerd

thehomebrewnerd commented Oct 28, 2021

Copy link
Copy Markdown
Contributor

@ridicolos I have uploaded new versions of the serialized files to S3. In order for your code to find these files, you will need to make the following updates:

In tests/entityset_tests/test_serialization, update _2021_8_31 in the TEST_FILE string to `_2021_10_28:

In tests/primitive_tests/test_feature_serailization.py, update _2021_8_31 in the TEST_FILE string to `_2021_10_28:

Once you make those updates, I will trigger the test runs to make sure all the unit tests pass.

@thehomebrewnerd

Copy link
Copy Markdown
Contributor

@ridicolos Sorry, I gave you some bad directions in my previous comment. I have updated to reflect the correct steps. Sorry for any confusion.

@ridicolos

Copy link
Copy Markdown
Contributor Author

@thehomebrewnerd No worrys. Haven't read the first version of your message before you changed that. 😉

I updated the TEST_FILE in both files in this commit c090cf2

@gsheni gsheni changed the title changed date_of_birth to birthday Change name of date_of_birth (column name) to birthday Nov 1, 2021
@gsheni gsheni changed the title Change name of date_of_birth (column name) to birthday Change name of date_of_birth (column name) to birthday in mock dataset Nov 1, 2021
@codecov

codecov Bot commented Nov 1, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1754 (9d5c2aa) into main (c9ca5dd) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1754      +/-   ##
==========================================
- Coverage   98.70%   98.64%   -0.07%     
==========================================
  Files         138      138              
  Lines       15389    15389              
==========================================
- Hits        15190    15180      -10     
- Misses        199      209      +10     
Impacted Files Coverage Δ
...utational_backend/test_calculate_feature_matrix.py 98.80% <ø> (-0.66%) ⬇️
featuretools/demo/mock_customer.py 95.23% <100.00%> (ø)
...etools/tests/entityset_tests/test_serialization.py 99.69% <100.00%> (ø)
...retools/tests/primitive_tests/test_feature_base.py 100.00% <100.00%> (ø)
...ests/primitive_tests/test_feature_serialization.py 99.35% <100.00%> (ø)
...ols/tests/synthesis/test_deep_feature_synthesis.py 99.32% <100.00%> (ø)
featuretools/tests/testing_utils/mock_ds.py 100.00% <100.00%> (ø)
...computational_backends/calculate_feature_matrix.py 98.30% <0.00%> (-0.85%) ⬇️
featuretools/tests/conftest.py 99.59% <0.00%> (-0.41%) ⬇️

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 c9ca5dd...9d5c2aa. Read the comment docs.

@thehomebrewnerd thehomebrewnerd 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 to me!

@rwedge
rwedge merged commit 018c271 into alteryx:main Nov 1, 2021
@gsheni

gsheni commented Nov 1, 2021

Copy link
Copy Markdown
Contributor

Congrats on your first merge request, @ridicolos !

This was referenced Nov 1, 2021
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.

Change name of date_of_birth to birthday

5 participants