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

Initial Type hinting: setting things up to gradually enable stricter type hinting #233

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

defron
Copy link
Contributor

@defron defron commented Apr 6, 2025

Initial Type hinting: setting things up to gradually enable stricter type hinting as part of #229

  • Individual changes are grouped together on commits. If there are any objections to any changes it should be easy to revert that specific commit
  • added dev dependency "types-Flask" for Flask types stubs
  • added to tool.pyright relaxed type enforcement settings
  • cleaned up ans sorted imports, removing unused imports to make pyright happy
  • added some basic type hints on areas that it made sense, as well as added some casting to help pyright know what types things are when ambiguous
  • added pyright ignore on use of private vars in overwritten mistune code indent method
  • added pyright ignore to the import of otterwiki.views since the import itself is adding the routes and views
  • handfle some possible None scenarios returned from gitpython related to blame, split log lines to be stored in a separate variable to keep type hints clean

Notes:

I'd like to slowly flip on the pyright settings set to false to warn/error based on what makes sense as type hinting work continues.

There are two cases where pyright shows an error currently because I didn't want to make a code change and didn't want to relax the rule, both are related to the rule reportUnnecessaryComparison:

*otterwiki/wiki.py L267: commit_count is explicitly set to 100 on line 235 and never changed, so it can never be None

  • otterwiki/sidebar.pyL175: path will always be at worst an empty string (confirming this was where most of the current type hints are from) so never None

Tests:

============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-8.3.5, pluggy-1.5.0
rootdir: /home/defron/src/otterwiki
configfile: pyproject.toml
collected 187 items

tests/test_attachments.py .........                                      [  4%]
tests/test_auth.py .............................                         [ 20%]
tests/test_draft.py ...                                                  [ 21%]
tests/test_essentials.py ....                                            [ 24%]
tests/test_gitstorage.py ......................                          [ 35%]
tests/test_helper.py ...............                                     [ 43%]
tests/test_otterwiki.py .....................                            [ 55%]
tests/test_preferences.py .............                                  [ 62%]
tests/test_preview.py .....                                              [ 64%]
tests/test_remote.py ......                                              [ 67%]
tests/test_renderer.py .....................................             [ 87%]
tests/test_settings.py ...                                               [ 89%]
tests/test_sidebar.py ....                                               [ 91%]
tests/test_util.py ................                                      [100%]

============================= 187 passed in 15.98s =============================

defron added 6 commits April 6, 2025 09:55
* add types stubs for flask in dev dependencies
* explicitly turn on pyright type checking with fairly relaxed rules to begin with
the code relies on the side-effect from the execution of the module on import adding the routes and views to flask so it is a false-positive

Also ignore storage being Unbound: fatal_error kills teh application so it will never be unbound
also store the array of log lines in a separate variable from original logentry so no variables change types
@redimp
Copy link
Owner

redimp commented Apr 6, 2025

Hey @defron, wow .. impressive work! Thank ypu. Will review and merge this in the next days.

@defron
Copy link
Contributor Author

defron commented Apr 7, 2025

sounds good! If you have any questions or concerns I'll try to answer them here as soon as I get a chance!

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