search

isi-vista / immutablecollections

A library for immutable collections, in the spirit of Guava's Immutable Collections.
MIT License
3 stars 2 forks source link

Implicit Any in `immutabledict` throws error on vistautils#86 #72

Open jamart28 opened 4 years ago

jamart28 commented 4 years ago

Implicit Any in immutabledict throws error on vistautils#86

Tracing problem through imports to find issue

jamart28 commented 4 years ago

It seems like the issue was that the aliases were expecting something passed through as they were being considered generics. As such one fix I'm currently trying is to not use the aliases. However this presents the following errors in vistautils (after installing the update version locally and using it): image Any ideas, Jacob (I couldn't find you in the @)?

jamart28 commented 4 years ago

Found the issue. Aliasing generic typings makes the alias itself generic and thus it uses implicit Any making the generic typing not work as intended. Furthermore, when importing just immutabledict, it does not like that pre-existing TypeVar's are used, instead favoring for the TypeVar's to be created in the typing if that makes sense. The necessary changes have been made and marked for review.

gabbard commented 4 years ago

@jamart28 : Can you provide me with more context of that the aliases are and where they are being used?

jamart28 commented 4 years ago

https://github.com/isi-vista/immutablecollections/blob/df73582691b1b989d96d61455804aa8e15009cf1/immutablecollections/_immutabledict.py#L20-L37 Excuse if my wording of alias wasn't exactly right. That's all I could think to call it.

gabbard commented 4 years ago

@jamart28 : ah, okay, so is the problem the type alias AllowableSourceType?

gabbard commented 4 years ago

@jamart28 : https://github.com/python/mypy/issues/606 implies type aliases with generics should work. Can you check what release of mypy it was added in vs. what release of mypy we are using?

jamart28 commented 4 years ago

@gabbard: That issue (and subsequent PR that allowed the generics) are from 2016 whereas the version we are using is from 2018. As such I think it's safe to say that the change is in the version we use. I believe that the issue comes in that we are importing it but am unsure how to workaround that (given that my solution I thought I had found causes issues in immutablecollections)

jamart28 commented 4 years ago

I should also clarify that the issue in the above code is that AllowableSourceType is technically a generic and needs types declared otherwise it gets an implicit Any. Using the KT and VT again we get the errors shown above.

jamart28 commented 4 years ago

@gabbard @lichmaster98 Looking through I believe our best course of action that will make the change the least noticeable in use of the library by others will be to take the implicit Any that is already on AllowableSourceType in the following line and make it explicit with AllowableSourceType[Any, Any]. Let me know if this fix is acceptable. https://github.com/isi-vista/immutablecollections/blob/df73582691b1b989d96d61455804aa8e15009cf1/immutablecollections/_immutabledict.py#L36

lichtefeld commented 4 years ago

@jamart28 From my understanding of the original issue using an explicit Any should be ok as we are trying to eliminate implicit ones. That would be my recommendation.