-
Notifications
You must be signed in to change notification settings - Fork 28
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
#3560 review #3663
#3560 review #3663
Conversation
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.
Other than one little nit (Tuple vs tuple), this all looks great.
Can we just merge it into the original PR, and let me continue from there (debugging the recent failure)?
this allows the type system to understand that e.g. the return value of a `.read()` can be a `PyDictLedgerEntry`, which can then have `to_dict()` invoked
40c4fa3
to
5e876ab
Compare
Fixed!
Yea, if you merge this, it will update bkm/hypothesis and you can continue working there |
* hypothesis draft * backport to python 3.9 and pandas<2.0 * remove metadata work-arounds * add metadata time travel ledger * add readme * fix numeric overflow in fastercsx test * remove sensitivity to another numeric precision corner case * increase scope of fastercsx coords tested * add string/binary columns to dataframe tests * lint * add enum/dict * disable io * fix padding required for arrow offset testing * remove obsolete comment * fix overflow warning * update readme * lint * remove sc-61239 work-around * remove incomplete test stub * #3560 review (#3663) * factor README link * use `mode: OpenMode` instead of `mode: str` * add missing `self` params * OpenMode * rm unused `SOMAArrayStateMachine._reopen` * `s/work-arounds/workarounds/` * add/tweak type annotations * `get_entries`: return `set[str]` * parameterize `Ledger` with a `LedgerEntryType` this allows the type system to understand that e.g. the return value of a `.read()` can be a `PyDictLedgerEntry`, which can then have `to_dict()` invoked * rm unused `concurrency` fixture * rm unused imports * avoid `st` shadowing --------- Co-authored-by: Ryan Williams <[email protected]>
Issue and/or context: stacked on #3560
I chased a few yellow/red squigglies in my editor, and also ran
mypy
onapis/python/tests/ht
and skimmed the issues it flagged (just as an audit;mypy
is normally disabled fortests/
because it generates too many FPs).Changes:
Added/Tweaked some type annotations, removed a few unused functions, etc.