-
Notifications
You must be signed in to change notification settings - Fork 97
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
chore(hf): remove unused arguments #556
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #556 +/- ##
=======================================
Coverage 87.87% 87.87%
=======================================
Files 96 96
Lines 9873 9873
Branches 1349 1349
=======================================
Hits 8676 8676
Misses 857 857
Partials 340 340
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying datachain-documentation with Cloudflare Pages
|
|
||
|
||
def _feature_to_chain_type(name: str, val: Any) -> type: # noqa: PLR0911 | ||
def _feature_to_chain_type(name: str, val: Any) -> DataType: # noqa: PLR0911 | ||
if isinstance(val, Value): | ||
return arrow_type_mapper(val.pa_type) |
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.
I see arrow_type_mapper
also returns type
but seems like it should also return DataType
, right? Maybe worth including in the PR. I think this stems from arrow_type_mapper
predating DataType
.
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.
yes, more cleanup / review is needed there with types
e.g. we return list(list ...
and it's not really DataType
AFAIU
it's not a complete solution, thus we still have a lot of ignore type pragmas
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.
Good catches @shcheklein!
* chore(hf): remove unused arguments * cleanup types a bit in hf
Minor cleanup in HF integration code.