GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — ci: check types with mypy by muggenhor · Pull Request #1226 · gitpython-developers/GitPython

Conversation

@muggenhor

@muggenhor muggenhor commented Apr 23, 2021

Copy link
Copy Markdown
Contributor

This is intended to (partially) address the CI concerns expressed in #1219 and #1223 and will result in partial type checking of the type annotations by using mypy. Keep in mind though that mypy is performing best-effort static analysis in a dynamically typed language so it can only partially check for correctness.

Some other tool(s) will be needed to have more complete type checking at runtime. E.g. typeguard.

In order to get mypy running successfully this also makes a few (as little as possible) changes to the type annotations. The mypy setting to require annotations on everything is commented out because there's way too much functions lacking annotations right now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Byron reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@muggenhor muggenhor changed the title ci: check types with mypy on CI ci: check types with mypy Apr 23, 2021
@Byron

Byron commented Apr 23, 2021

Copy link
Copy Markdown
Member

Thanks a lot for taking this on! Your effort is much appreciated, and also much needed to get GitPython back on track.

It appears that thanks to never actually running the type checker, the previous commits to add types weren't validated and seemingly aren't working (or are only working with python 3.9).

I have no idea how much additional work it would be to get mypy to pass, but if it's a dead-end it would certainly be alright to give up on types altogether and revert the commits that added them.

It's all up to you.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@muggenhor

muggenhor commented Apr 23, 2021

Copy link
Copy Markdown
Contributor Author

Combined with #1227 this should pass now.
Byron reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
muggenhor added 9 commits April 23, 2021 15:23
@muggenhor
test: sort MANIFEST.in and add missing test-requirements.txt
70aa1ab 
Without the presence of 'test-requirements.txt' 'tox' is unusable.
@muggenhor
test(tox): verify type annotations
4f9d492
@muggenhor
test(mypy): don't give errors for every unannotated function
6d06f5a 
Because there's too many to fix quickly.
@muggenhor
test(mypy): don't complain about missing type hints for 'gitdb'
74a1b17
@muggenhor
fix(mypy): properly describe link between parameter and return types
6a23335 
This gives mypy all information that it needs to determine what the
return type of a function call is *iff* it knows the argument's type.

As a result it can now stop complaining about passing None to str.join()
in exc.py.
@muggenhor
improvement: teach mypy that Object.type is not always supposed to be…
c885781 
… None
@muggenhor
improvement: teach mypy how to deal with wildcard-imported objects
8470777 
By telling it where it's imported from in one case and telling it to
ignore it in another.
@muggenhor
improvement(mypy): ignore false positives
76ba092
@muggenhor
ci: check types with mypy
043e15f 
This will result in _partial_ type checking of the type annotations by
using mypy. Keep in mind though that mypy is performing _static_
analysis in a _dynamic_ language so it can only partially check for
correctness.

Some other tool(s) will be needed to have more complete type checking at
runtime. E.g. [typeguard].

[typeguard]: https://pypi.org/project/typeguard/
@muggenhor

muggenhor commented Apr 23, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Rebased (to force test-rerun) after #1227 got merged.

Unfortunately it looks like the 3.9 build failed due to what looks like an intermittent failure. And I cannot retrigger a build without (another) force push....
Byron reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Byron
Byron merged commit 4119a57 into gitpython-developers:main Apr 23, 2021
@Byron

Byron commented Apr 23, 2021

Copy link
Copy Markdown
Member

Thanks a million!
muggenhor reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@muggenhor
muggenhor deleted the testing branch April 23, 2021 13:39
@Byron Byron mentioned this pull request Apr 23, 2021
[tracking] Unbreaking v3.1.15 (yanked) #1223
Closed
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@muggenhor @Byron
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.