Protecting our API with beartype #227
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 39 39
Lines 2447 2439 -8
Branches 335 331 -4
=========================================
- Hits 2447 2439 -8 ☔ View full report in Codecov by Sentry. |
benoit74
force-pushed
the
safe_metadata_revamp
branch
from
a2c1456 to
7e2efa1
Compare
This enables beartype on all of scraperlib, raising exceptions on all calls to our API that violates the requested types (and function returning incorrect types) Changes: - removed tests purposedly testing incorrect input types - fixed some return types or call params to match intent - logger console arg expecting io.StringIO - turned a couple NamedTuple into dataclass to ease with type declarations - Removed some unreachable code that was expecting invalid types - Introducing new protocols for IO-based type inputs, based on typeshed's (as protocols, those are ignored in test/coverage) - Image-related IO are declared as io.BytesIO instead of `IO[bytes]`. Somewhat equivalent but typing.IO is strongly discouraged everywhere. Cannot harmonize with rest of our code base as we pass this to Pillow. - Same goes for logger which eventually accepts TextIO - stream_file behavior changed a bit. Code assumed that if fpath is not there we want byte_stream. Given it's not properly tested, I changed it to accept both fpath and byte_stream simultaneously. I believe we should change the API to have a single input that supports the byte stream and the path and adapt behavior. We should do that through the code base though so that would be separate. I'll open a ticket if we agree on going this way
Three blocks of code had to be marked no cover. Those blocks are entered and tested but coverage reports them as missing. It might be due to the decorator… I've tried to simplify the code leading to it but could fix the missing lines… Once we merge this, I'll open a ticket to revisit this in the future.
We should really ban wide `pyright: ignore` statements and use specifix expections wherever necessary (or comply!) This fixes it in the current codebase
benoit74
force-pushed
the
metadata_and_beartype
branch
from
237e4a4 to
6f93ffe
Compare
benoit74
approved these changes
Dec 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This targets #221 and build ont it. Here's the recap of the commits:
Using beartype on all of scraperlib
This enables beartype on all of scraperlib, raising exceptions on all calls to our API
that violates the requested types (and function returning incorrect types)
Changes:
IO[bytes]. Somewhat equivalent but typing.IO is strongly discouraged everywhere. Cannot harmonize with rest of our code base as we pass this to Pillow.--
Disable cover on special blocks
Three blocks of code had to be marked no cover. Those blocks are entered and tested
but coverage reports them as missing. It might be due to the decorator…
I've tried to simplify the code leading to it but could fix the missing lines…
Once we merge this, I'll open a ticket to revisit this in the future.
--
Removed generic pyright: ignore statements
We should really ban wide
pyright: ignorestatements and use specifix expectionswherever necessary (or comply!)
This fixes it in the current codebase