Introducing Type Hints #199
Merged
Conversation
This file contains 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
As we are producing a binary extension via Cython, the regular source-based mechanism of type discovery is not available. This makes using libzim a little less pleasant for those relying on types. This adds type stubs that we will have to manually maintain. I had to tweak a bit the wrapper to be able to expose the get_indexdata thing. Previously, we were relying on the fact that the Item had no `get_indexdata` method to decide whether to use the libzim auto-index feature (relied on heavily for HTML entries). Conditional method is not really compatible with static typing. In order to retain functionality and API, I chose to add a `get_indexdata` variable on all `Item`, set to `None`. If this variable is `None` (no action taken), then we run the auto-index. If it is not None but returns NULL, then no index (no auto neither) If it is set and returns a proper IndexData, it is used. If the variable is missing (not sub-classing `Item`), run the auto-index. In other words, behavior stays the same but type checker have a `get_indexdata` variable to look-for.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #199 +/- ##
==========================================
+ Coverage 93.44% 93.49% +0.05%
==========================================
Files 1 1
Lines 519 523 +4
==========================================
+ Hits 485 489 +4
Misses 34 34 ☔ View full report in Codecov by Sentry. |
benoit74
approved these changes
Sep 9, 2024
Closed
Closed
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.
As we are producing a binary extension via Cython, the regular source-based mechanism of type discovery is not available. This makes using libzim a little less pleasant for those relying on types.
This adds type stubs that we will have to manually maintain.
I had to tweak a bit the wrapper to be able to expose the get_indexdata thing. Previously, we were relying on the fact that the Item had no
get_indexdatamethod to decide whether to use the libzim auto-index feature (relied on heavily for HTML entries).Conditional method is not really compatible with static typing.
In order to retain functionality and API, I chose to add a
get_indexdatavariable on allItem, set toNone. If this variable isNone(no action taken), then we run the auto-index. If it is not None but returns NULL, then no index (no auto neither) If it is set and returns a proper IndexData, it is used. If the variable is missing (not sub-classingItem), run the auto-index.In other words, behavior stays the same but type checker have a
get_indexdatavariable to look-for.I don't see it as an API change and thus plan on including in 3.5.0 but please let me know what you thin
Nota: no test behavior change. changes are only to please type-checker or to silence it in cases we were not respecting it on purpose.