Skip to content

Add type annotations for punq#226

Open
sobolevn wants to merge 6 commits into
mainfrom
add-type-annotations
Open

Add type annotations for punq#226
sobolevn wants to merge 6 commits into
mainfrom
add-type-annotations

Conversation

@sobolevn

Copy link
Copy Markdown
Collaborator

No description provided.

@sobolevn sobolevn requested a review from bobthemighty April 19, 2026 10:44
@codecov

codecov Bot commented Apr 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.0%. Comparing base (6794f45) to head (09f9295).

Files with missing lines Patch % Lines
punq/__init__.py 95.7% 2 Missing and 1 partial ⚠️

❌ Your project check has failed because the head coverage (82.0%) is below the target coverage (90.0%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff            @@
##             main    #226      +/-   ##
=========================================
- Coverage   100.0%   82.0%   -18.0%     
=========================================
  Files           2       1       -1     
  Lines         205     201       -4     
  Branches       25      28       +3     
=========================================
- Hits          205     165      -40     
- Misses          0      24      +24     
- Partials        0      12      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sobolevn

sobolevn commented Apr 19, 2026

Copy link
Copy Markdown
Collaborator Author

@bobthemighty I am leaving failing coverage checks, because I am pretty sure that it is not correct. I've double checked that running tests and adding exceptions to the "uncoverred" lines actually fail tests:

self = <punq.Container object at 0x1060de050>
service_key = <class 'tests.test_dependencies.MessageWriter'>, registration = None

    def _should_auto_register(self, service_key: Any, registration: _Registration[Any] | None) -> bool:
        if self._auto_register is False:
            return False
    
>       raise ValueError
E       ValueError

punq/__init__.py:596: ValueError
================================= short test summary info =================================
FAILED tests/test_auto_registration.py::test_when_requesting_an_initialisable_object_that_is_not_in_the_container - ValueError
FAILED tests/test_auto_registration.py::test_when_requesting_an_uninitialisable_object_that_is_not_in_the_container - ValueError
FAILED tests/test_auto_registration.py::test_the_child_of_an_auto_registering_container_is_auto_registering - ValueError
FAILED tests/test_instance_creation.py::test_can_create_instance_with_auto_register - ValueError
============================== 4 failed, 47 passed in 0.13s ===============================
                          

So, I consider this ready for review.

@bobthemighty bobthemighty left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to approve if this works for type checks and subtyping. Can we add a block at the bottom of the test container module

if typing.TYPE_CHECKING:
  foo : Impl = container.resovle(...)

for a bunch of examples? Looks pretty good to me. Maybe I was just struggling with 3.8/3.9?


assert_type(container.resolve(Service), Service)
assert_type(container.resolve("string"), Any)
assert_type(container.resolve(Impl, arg=1, other=2), Impl)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all look concrete to me, how does this work with subtyping?

container.register(MessageWriter, StdoutMessageWriter)

assert_type(container.resolve(MessageWriter, MessageWriter)

Also, I'm not completely familair with the type testing lib. Does this implementation type check

foo: MessageWriter = container.resolve(MessageWriter)

If so, I'm impressed this was so straightforward!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added several examples 👍

Comment thread pyproject.toml Outdated
version = "0.7.1"
description = "An IOC Container for Python 3.8+"
description = "An IOC Container for Python 3.10+"
authors = [{ name = "Bob Gregory", email = "bob@codefiend.co.uk" }]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you missed an edit.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

@sobolevn

Copy link
Copy Markdown
Collaborator Author

I am going to merge this in a couple of days, if everything is good :)

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