Fix Fideslang Typing #3839
Fix Fideslang Typing #3839
Conversation
Passing run #4267 ↗︎
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
||||||||||||||||||||
ThomasLaPiana
added
the
run unsafe ci checks
|
update here: looks like everything is good except for the unsafe checks. I suspect at least the |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #3839 +/- ##
===========================================
+ Coverage 64.96% 87.51% +22.54%
===========================================
Files 326 326
Lines 20250 20283 +33
Branches 2622 2634 +12
===========================================
+ Hits 13155 17750 +4595
+ Misses 6629 2076 -4553
+ Partials 466 457 -9
☔ View full report in Codecov by Sentry. |
|
@galvana can you give me confirmation that the failing tests here are expected? I looked at Note: The failing ctl external test is a known issue |
ThomasLaPiana
requested review from
pattisdr,
adamsachs and
TheAndrewJackson
There was a problem hiding this comment.
great @ThomasLaPiana thanks for the work to allow us to return on automated mypy checks again! so useful
| @@ -43,7 +43,6 @@ module = [ | |||
| "dask.*", | |||
| "deepdiff.*", | |||
| "defusedxml.ElementTree.*", | |||
| "fideslang.*", | |||
There was a problem hiding this comment.
this section tells mypy to ignore missing type imports from imported libraries. Everything from fideslang is typed so this is redundant
at least that's my understanding
|
|
||
| FieldDataCategoryValidation.update_forward_refs() | ||
|
|
||
| class CollectionDataCategoryValidation( | ||
| DatasetCollection, DataCategoryValidationMixin | ||
| ): | ||
| fields: List[FieldDataCategoryValidation] = [] | ||
| fields: Sequence[FieldDataCategoryValidation] = [] # type: ignore[assignment] |
There was a problem hiding this comment.
Where is this coming from?
There was a problem hiding this comment.
This is required due to Lists in Python being invariant, as a result of their mutability
There was a problem hiding this comment.
thank you for not losing sight of this and staying diligent with maintenance! nothing stands out to me as particularly problematic. i do think @pattisdr had some good questions that may be nice to get answers to before we merge.
🎊
EDIT -- just wanted to note a reminder that we need to follow up with fidesplus - both to make any needed changes based on dependencies on what we've changed here (not sure if there's anything applicable), and just generally getting mypy fixed there! i'll see if i can help push that forward once this is merged 👍
Closes #N/A
Description Of Changes
I inadvertently fixed typing over in
fideslang, which means we now have a bunch of things to fix here!This uncovered a lot of legitimate issues in our type annotations.
Code Changes
mypypassingSteps to Confirm
Pre-Merge Checklist