Repository navigation
Refactor typings - #62
Conversation
|
I thought about this a bit more and one advantage of disallowing different key and value types is that |
|
I'm strongly in favor of restricting |
I think I've convinced myself of restriction as well. I'll update the PR. |
|
I've updated the PR making from immutables import Map
from typing import Dict, Union, cast
def init() -> None:
def thing(m: Map[str, Union[str, int]]) -> None:
...
thing(Map(foo=1))
thing(Map(foo='bar', baz=1))
thing(Map([('foo', 'bar'), ('bar', 1)]))
thing(Map(Map(foo=1), bar='foo'))
m = Map({1: 2})
thing(m) # Incompatible type
def assignments() -> None:
m_int__str = Map[int, str]()
m_str__str = Map[str, str]()
m_int_str__str = Map[Union[int, str], str]()
m_str__int_str = Map[str, Union[int, str]]()
m_int__str = m_str__str # Incompatible types
m_int__str = m_int_str__str # Incompatible types
m_int__str = m_str__int_str # Incompatible types
m_str__str = m_int__str # Incompatible types
m_str__str = m_int_str__str # Incompatible types
m_str__str = m_str__int_str # Incompatible types
m_int_str__str = m_int__str # Incompatible types
m_int_str__str = m_str__str # Incompatible types
m_int_str__str = m_str__int_str # Incompatible types
m_str__int_str = m_int__str # Incompatible types
m_str__int_str = m_int_str__str # Incompatible types
m_str__int_str = m_str__str
def update() -> None:
m_int__str: Map[int, str] = Map()
m_str__str: Map[str, str] = Map()
m_int_str__str: Map[Union[int, str], str] = Map()
m_str__int_str: Map[str, Union[int, str]] = Map()
m_int__str.update({1: '2'})
m_int__str.update({1: '2'}, three='4') # Unexpected keyword argument
m_int__str.update({1: 2}) # Argument 1 has incompatible type
m_str__str.update({'1': '2'})
m_str__str.update({'1': '2'}, three='4')
m_str__str.update({'1': 2}) # Argument 1 has incompatible type
m_int_str__str.update(cast(Dict[Union[int, str], str], {1: '2', '3': '4'}))
m_int_str__str.update({1: '2'}, three='4')
m_int_str__str.update({'1': 2}) # Argument 1 has incompatible type
m_str__int_str.update({'1': 2, '2': 3})
m_str__int_str.update({'1': 2, '2': 3}, four='5')
m_str__int_str.update({1: 2}) # Argument 1 has incompatible type
def mutate() -> None:
m = Map[str, str]()
with m.mutate() as mm:
mm[0] = '1' # Invalid index type
mm['1'] = 0 # Incompatible types
mm['1'] = '2'
del mm['1']
mm.set('3', '4')
m2 = mm.finish()
reveal_type(m2) # Revealed type is 'immutables._map.Map[builtins.str*, builtins.str*]' |
| recursive-include immutables *.py *.c *.h *.pyi | ||
| include LICENSE README.rst | ||
| include immutables/py.typed | ||
| include mypy.ini immutables/py.typed |
There was a problem hiding this comment.
What's the reason for adding mypy.ini in the distribution?
There was a problem hiding this comment.
I think I did this out of habit. Most of the projects I work on do some sort of type checking during testing. That doesn't seem to be the case for this one.
There was a problem hiding this comment.
Actually, I think it is good idea to add such a test, otherwise the annotations will inevitably rot.
There was a problem hiding this comment.
@elprans would you like me to add a test that runs mypy?
|
Hi, can I somehow help to get this to master ? |
4824050 to
5e86789
Compare
elprans
left a comment
There was a problem hiding this comment.
@msullivan, thanks! Hopefully we'll be able to make mypy more amenable to remove the need for these hacks in the future.
* Improve typing of `__init__()` * Update typing of `Map`-producing functions to produce the correct type * Update typing of other methods to more closely align with `Mapping` * Add protocol classes for unexposed data structures * Export protocol classes for ease of use in typed code * Update stub file to pass in mypy strict mode
651fec2 to
99417a0
Compare
__init__()Map-producing functions to produce the correct typeMappingfixes #54
I've taken the work in the typing overhaul PR and improved it as detailed above. The following passes using
mypyin strict mode:There was discussion in #54 about restricting
update()and otherMap-producing functions to the key and value types of the originalMap. I'm not exactly sure which is the best approach since the implementation allowsupdate()to take different types for the key and value, howevertyping.Dictdisallows this fordicteven thoughdict.update()allows similar functionality asMap.update(). For now, I've leftMap.update()similar to what was done in #54, but that can certainly be changed before merging.