chore: Delete unused code in entity - #3008
Conversation
Signed-off-by: Felix Wang <wangfelix98@gmail.com>
Signed-off-by: Felix Wang <wangfelix98@gmail.com>
Signed-off-by: Felix Wang <wangfelix98@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #3008 +/- ##
==========================================
+ Coverage 77.83% 78.01% +0.17%
==========================================
Files 194 193 -1
Lines 16567 16442 -125
==========================================
- Hits 12895 12827 -68
+ Misses 3672 3615 -57
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
6989d46 to
f453ff7
Compare
Signed-off-by: Felix Wang <wangfelix98@gmail.com>
| schema=[ | ||
| Field(name="temperature", dtype=Int32) | ||
| Field(name="temperature", dtype=Int32), | ||
| Field(name="location", dtype=Int64), |
There was a problem hiding this comment.
shouldn't this be location_id?
| Attributes: | ||
| name: The unique name of the entity. | ||
| value_type (deprecated): The type of the entity, such as string or float. | ||
| value_type: The type of the entity, such as string or float. |
There was a problem hiding this comment.
value type should be gone right?
There was a problem hiding this comment.
value_type is still an attribute of an entity; it's just not set during initialization, but instead inferred during type inference
| join_keys: A list of properties that uniquely identifies different entities within the | ||
| collection. This currently only supports a list of size one, but is intended to | ||
| eventually support multiple join keys. | ||
| join_key: A property that uniquely identifies different entities within the |
There was a problem hiding this comment.
we haven't removed the usage of join_key throughout the codebase, so we still want to be able to do entity.join_key, which is why join_key is left as an attribute
I clarified the TODO below
| dob_ssn = Entity( | ||
| name="dob_ssn", | ||
| value_type=ValueType.STRING, | ||
| description="Date of birth and last four digits of social security number", |
There was a problem hiding this comment.
can you update the rest of the FCOs? the FVs that depend on this weren't updated
|
|
||
| test-python: | ||
| FEAST_USAGE=False IS_TEST=True python -m pytest -n 8 sdk/python/tests | ||
| FEAST_USAGE=False IS_TEST=True python -m pytest -n 8 sdk/python/tests -k "not test_sql_registry" |
There was a problem hiding this comment.
why did you exclude this test? Can you add a comment?
There was a problem hiding this comment.
added comment
There was a problem hiding this comment.
actually, just marked the test_sql_registry tests as integration tests since they require external services
There was a problem hiding this comment.
actually after talking with Achal, we decided to make it a unit test; will do in a separate PR
There was a problem hiding this comment.
These should be able to run as unit tests - any reason they can't?
There was a problem hiding this comment.
after discussing, we agreed to make these unit tests; will do in separate PR
Signed-off-by: Felix Wang <wangfelix98@gmail.com>
b2a9cdd to
1e028b0
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: achals, felixwang9817 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #