scott-griffiths / bitstring

A Python module to help you manage your bits
https://bitstring.readthedocs.io/en/stable/index.html
MIT License
404 stars 68 forks source link

Fix typehint for derived Bits classes #277

Closed meggiman closed 1 year ago

meggiman commented 1 year ago

This PR fixes #276 using the PEP 673 Self type as the hinted return value type.

scott-griffiths commented 1 year ago

Hi, thanks for the fix. I was actually looking at this just yesterday and read about the Self type for the first time, so it's useful to see it working.

Adding this would add a dependency on the external typing_extensions module, which I'm hesitant to do (perhaps I shouldn't be, but the next version of bitstring will be the first with any dependencies and I was hoping to keep it to just the one).

I'll take a look at what's the minimal I need to do when I get the chance, but a fix should go in for the next versions.

scott-griffiths commented 1 year ago

(The previous comment was meant for the ticket not the PR!)

meggiman commented 1 year ago

I see the argument for not introducing additional dependencies. In that case, how about using a TypeVar-based solution? I.e. something like this:

TBits = TypeVar("TBits", bound='Bits')
def __new__(cls: Type[TBits], auto: Optional[BitsType] = None, length: Optional[int] = None,
      offset: Optional[int] = None, pos: Optional[int] = None, **kwargs) -> TBits:
      ...

This would not require any external dependency.

scott-griffiths commented 1 year ago

Yes, the TypeVar was what I was looking at. In particular the answer here which is buggy (the class is referenced before it's defined) but your code looks better. So yes, this would be an good fix if you want to make a new PR.

scott-griffiths commented 1 year ago

I've now added the TypeVar code to the main branch. Thanks for your help.