Added data check action for setting first column as primary key if identified as ID #3634
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #3634 +/- ##
=======================================
+ Coverage 99.7% 99.7% +0.1%
=======================================
Files 335 335
Lines 33638 33666 +28
=======================================
+ Hits 33515 33543 +28
Misses 123 123
Help us with your feedback. Take ten seconds to tell us how you rate us. |
simha104
changed the title
docs/source/release_notes.rst
Outdated
| @@ -28,6 +28,7 @@ Release Notes | |||
| * Changes | |||
| * Add pre-commit hooks for linting :pr:`3608` | |||
| * Implemented a lower threshold and window size for the ``TimeSeriesRegularizer`` and ``DatetimeFormatDataCheck`` :pr:`3627` | |||
| * Datasets where the first column is identified as an id column now produce a recommended action to set the column as the primary key :pr:`3634` | |||
There was a problem hiding this comment.
Could you include the reference to the data check that was changed, so something like
"Updated IDColumnsDataCheck to return an action to set the first column as the primary key if it is identified as an ID column"
| ) | ||
| message_code = DataCheckMessageCode.HAS_ID_FIRST_COLUMN | ||
| action_code = DataCheckActionCode.SET_FIRST_COL_ID | ||
| details = {"column": col_names[0]} |
There was a problem hiding this comment.
I think this should be changed to columns
| message_code = DataCheckMessageCode.HAS_ID_FIRST_COLUMN | ||
| action_code = DataCheckActionCode.SET_FIRST_COL_ID | ||
| details = {"column": col_names[0]} | ||
| else: |
There was a problem hiding this comment.
Are we trying to make this an either/or scenario? I think we would still want to know if other columns could be ID columns right?
| "col_6": [0.1, 0.2, 0.3, 0.4], | ||
| } | ||
|
|
||
| X = pd.DataFrame.from_dict(X_dict) |
There was a problem hiding this comment.
I think we would expect col_3_id to also show up under DataCheckMessageCode.HAS_ID_COLUMN.
| ), | ||
| ], | ||
| ).to_dict(), | ||
| ] | ||
|
|
||
| # test np.array | ||
| assert id_cols_check.validate( | ||
| np.array([[0, 1], [1, 2], [2, 3], [3, 4], [4, 5]]), | ||
| np.array([[0, 1], [0, 2], [2, 3], [3, 4], [4, 5]]), |
There was a problem hiding this comment.
Why were some of these other tests changed?
bchen1116
approved these changes
Aug 1, 2022
| @@ -22,6 +22,9 @@ class DataCheckActionCode(Enum): | |||
| REGULARIZE_AND_IMPUTE_DATASET = "regularize_and_impute_dataset" | |||
| """Action code for regularizing and imputing all features and target time series data.""" | |||
|
|
|||
| SET_FIRST_COL_ID = "set_first_col_id" | |||
| """Action code for setting the first column as an id column""" | |||
| @@ -17,6 +17,9 @@ class DataCheckMessageCode(Enum): | |||
| HAS_ID_COLUMN = "has_id_column" | |||
| """Message code for data that has ID columns.""" | |||
|
|
|||
| HAS_ID_FIRST_COLUMN = "has_id_first_column" | |||
| """Message code for data that has an ID column as the first column""" | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Added data check action for setting first column as primary key if identified as ID