-
Notifications
You must be signed in to change notification settings - Fork 908
Fixed issue #947 by editing type strings #982
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
Conversation
…t files on s3. Documentation for future entity and schema changes provided here: https://alteryx.quip.com/c8LjAy7T2fuD/Making-changes-to-the-schema
Codecov Report
@@ Coverage Diff @@
## master #982 +/- ##
=======================================
Coverage 98.22% 98.23%
=======================================
Files 119 119
Lines 10901 10905 +4
=======================================
+ Hits 10708 10713 +5
+ Misses 193 192 -1
Continue to review full report at Codecov.
|
|
After this change, the |
|
Since we've changed the type string for I also think this edge case -- being unable to find a variable that matches the type string in a variable json -- is untested. We should add a test case. |
|
Let's also add a line to the breaking changes section of the changelog, detailing which Variables had their type strings changed. |
…ase where type_string was not found in the variable_types dict. Added a supporting test_case.
…ng change. Added the breaking change to the changelog
docs/source/changelog.rst
Outdated
| no longer used to determine the time column. Now, both instance id columns and time columns in a cutoff time | ||
| dataframe can be in any order as long as they are named properly. | ||
|
|
||
| * The type_string attributes of all entity classes are now a snake case conversion of their class names. This |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- variable classes, not entity classes
- let's use code formatting (``text``) around type_string and the formal class names (Unknown, IPAddress, etc)
- Instead of talking about schema versions, in the last sentence let's say that old saved entitysets that used these variable may load incorrectly
rwedge
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Pull Request Description