Skip to content
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

Merged
merged 12 commits into from
Feb 4, 2025
Merged

#3560 review #3663

merged 12 commits into from
Feb 4, 2025

Conversation

ryan-williams
Copy link
Member

Issue and/or context: stacked on #3560

I chased a few yellow/red squigglies in my editor, and also ran mypy on apis/python/tests/ht and skimmed the issues it flagged (just as an audit; mypy is normally disabled for tests/ because it generates too many FPs).

Changes:

Added/Tweaked some type annotations, removed a few unused functions, etc.

Copy link
Member

@bkmartinjr bkmartinjr left a 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
@ryan-williams
Copy link
Member Author

ryan-williams commented Feb 4, 2025

Other than one little nit (Tuple vs tuple), this all looks great.

Fixed!

Can we just merge it into the original PR, and let me continue from there (debugging the recent failure)?

Yea, if you merge this, it will update bkm/hypothesis and you can continue working there

@bkmartinjr bkmartinjr merged commit 6a8c746 into bkm/hypothesis Feb 4, 2025
1 check passed
@bkmartinjr bkmartinjr deleted the rw/3560 branch February 4, 2025 16:37
bkmartinjr added a commit that referenced this pull request Feb 5, 2025
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants