-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fixes for errors reported by mypy 1.11.0 #18608
Conversation
@jmchilton How would you like the errors in |
I would say we should just replace the very generic signatures with the specific ones and pray nothing breaks. Might hit some bugs in the mean time but long term that sounds better. I need to finish the dataset uuid stuff but if this is still red tomorrow - I could try to work through it. |
Thanks @jmchilton , I followed your suggestion but please double check! |
3c7d4df
to
849a091
Compare
75d097b
to
f821e3b
Compare
7511e95
to
398c1c7
Compare
@jmchilton Only 17 mypy issues left, and all other tests (except the unrelated integration test failures) are green. |
I think the fixes are higher priority than the mypy upgrade itself. Can I recommend we strip out the mypy upgrade and just PR the fixes so they don't stale. I'd be willing to take that task on while you're away - I am not ready to commit to finishing the upgrade though. |
It is never called like that so I assume value is always None in practice.
Sounds good, this part should be ready for review/merge. |
Wow - that is a lot of scary changes. I think we have really good test coverage for the object store stuff and that is what I'm most worried about. We also have good test coverage of the toolbox and manager things I think. I'm most worried about the data provider stuff but I walked through closely and I think it should be all fine and I'm not certain any of it is used at this point anyway. |
Integration test failures unrelated. |
This PR was merged without a "kind/" label, please correct. |
How to test the changes?
(Select all options that apply)
License