Skip to content

Add 2 new Type Hinting functions for helping with function or Protocol Styled approaches #705

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

Closed

Conversation

Vizonex
Copy link
Contributor

@Vizonex Vizonex commented Jun 3, 2025

What do these changes do?

My old pull request I tried doing the previous day to aiohttp just wouldn't cut it, and it felt a little embarrassing to have to close it and start over :/

There should first need to be a way to converse between 2 different Styles of writing functions as well as 2 different ways to send ParamSpec type hints through to a Signal so that it ides like pyright would know how to carry them. This way type hinting is kept retained and we don't need to create any extreme changes to TraceConfig on aiohttp such as overriding as member descriptors (maybe not the best approach here).

Wanted to make sure users could see what kind of parameters were being passed through pyright or mypy no matter what the circumstances were, therefore I had decided to make sure these 2 two function were also heavily documented, know that it's not perfect but it's a start.

I created 2 different approaches to solve passing ParamSpec Keywords.

Method 1 Passing via Function Signature

from aiosignal import signal_func

@signal_func
async def my_signal(a:int, b:int) -> None:...

# my_signal has been transformed to a Signal 
# and it should now be able to the see the typehints: 
#   (owner: object) -> Signal[(a: int, b: int), None]
s = my_signal(None)

Method 2 Passing via Protocol

Probably the one everyone maintaining aiohttp wanted to see...
I was primarily looking for a way to make the protocol manipulatable and it took me about 2 to 3
hours of research and attempts to get it right and since we can't use function generics like in Python 3.12
This was the most creative idea I could come up with. Type hints passed, users can see what to send through
TraceConfig , problem solved. The bonus is that using Generics makes the parameters manipulatable this is something
that aiohttp does with TraceConfig on a heavy scale. Type casting should also work as intended as well.

        from typing import TypeVar, Protocol

        A = TypeVar("A")
        B = TypeVar("B")

        class MySignalProtocol(Protocol[A, B]):
            @signal_method
            def __call__(self, a: A, b: B) -> None:...

        MySignal: MySignalProtocol[dict, int] = Signal

        # Signal is now been typehinted via protocol
        # pyright or other ides should be able to 
        # identify it as: 
        #   (owner:object) -> Signal[(a: dict, b: int), None]

        signal = MySignal()

Are there changes in behavior for the user?

There shouldn't be all I added were 2 new functions to ensure that users had a way to pass ParamSpec arguments to the Signal class that I had established as a result of reviving this library. This was the puzzle that needed solving. Having the full signature rather than a Callable would help users to see & identify different keyword arguments and types if any are to be passed.

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

Copy link

codecov bot commented Jun 3, 2025

Codecov Report

Attention: Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.23%. Comparing base (8789545) to head (1af8106).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
tests/test_signals.py 93.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #705      +/-   ##
==========================================
- Coverage   99.26%   98.23%   -1.03%     
==========================================
  Files           2        2              
  Lines         136      170      +34     
  Branches       11       13       +2     
==========================================
+ Hits          135      167      +32     
- Misses          0        2       +2     
  Partials        1        1              
Flag Coverage Δ
CI-GHA 98.23% <94.87%> (-1.03%) ⬇️
MyPy 60.00% <71.42%> (-1.54%) ⬇️
OS-Linux 98.23% <94.87%> (-1.03%) ⬇️
Py-3.10.17 97.63% <92.30%> (-0.89%) ⬇️
Py-3.11.12 97.63% <92.30%> (-0.89%) ⬇️
Py-3.12.10 97.63% <92.30%> (-0.89%) ⬇️
Py-3.13.3 98.23% <94.87%> (-1.03%) ⬇️
Py-3.9.22 97.63% <92.30%> (-0.89%) ⬇️
VM-ubuntu-24.04 98.22% <94.87%> (-1.04%) ⬇️
VM-ubuntu-latest 60.00% <71.42%> (-1.54%) ⬇️
pytest 98.22% <94.87%> (-1.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@Vizonex
Copy link
Contributor Author

Vizonex commented Jun 3, 2025

Can't figure out what I did wrong to get aiosignal/__init__.py:82:89: E501 line too long (89 > 88 characters)
I'll try again later when I get more time on my hands...

@Vizonex
Copy link
Contributor Author

Vizonex commented Jun 4, 2025

I don't think coverage likes my new changes this time, this could be a problem...

@Vizonex
Copy link
Contributor Author

Vizonex commented Jun 4, 2025

image
Works just fine on my ide even. Coverage might be throwing a fit because the wrapper attempts to throw the function away and then trades off ParamSpec for syntax sugaring. I wonder if that might be the problem?

@Vizonex
Copy link
Contributor Author

Vizonex commented Jun 7, 2025

No need for this one anymore I solved it :)
aio-libs/aiohttp#11160

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.

1 participant