Skip to content
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

Corrected special attributes present in object or its direct inheritors #874

Closed

Conversation

vlasovskikh
Copy link
Member

No description provided.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Maybe you can separate the non-controversial things out of this so we can merge those separately?

(Also, I realize that PyCharm's needs for typeshed are different from mypy's. Maybe we need to add some way to differentiate, similar to checks for sys.version_info and sys.platform? If you like that idea we should probably discuss this in the typing repo first.)

@@ -30,26 +30,27 @@ class classmethod: pass # Special, only valid as a decorator.
class object:
__doc__ = ... # type: Optional[str]
__class__ = ... # type: type
__dict__ = ... # type: Dict[str, Any]
Copy link
Member

Choose a reason for hiding this comment

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

This isn't really correct -- object itself doesn't have __dict__ and neither do classes using __slots__.


def __init__(self) -> None: ...
def __new__(cls) -> Any: ...
def __setattr__(self, name: str, value: Any) -> None: ...
def __eq__(self, o: object) -> bool: ...
def __ne__(self, o: object) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

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

Strange you're deleting these, because object does have __eq__ and __ne__.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's true for type objects like object class, but as for object() instances I'm not sure about it:

>>> object().__eq__
Traceback (most recent call last):
  ...
AttributeError: 'object' object has no attribute '__eq__'
>>> object().__ne__
Traceback (most recent call last):
  ...
AttributeError: 'object' object has no attribute '__ne__'

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good catch. Looks like a Python 2/3 compatibility issues -- in Python 3 they exist:

>>> object().__eq__
<method-wrapper '__eq__' of object object at 0x10062e110>

Copy link
Member

Choose a reason for hiding this comment

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

Also, while the methods may not exist, the operations from which they are typically used (== and !=) do work on objects.

Copy link
Member

Choose a reason for hiding this comment

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

(And I just confirmed that mypy fails simple correct code like this:

class A: pass
A() == A()

with the error

__tmp__.py:2: error: Unsupported left operand type for == ("A")

@@ -482,6 +483,8 @@ class tuple(Sequence[_T_co], Generic[_T_co]):
def __le__(self, x: Tuple[_T_co, ...]) -> bool: ...
def __gt__(self, x: Tuple[_T_co, ...]) -> bool: ...
def __ge__(self, x: Tuple[_T_co, ...]) -> bool: ...
def __eq__(self, x: Tuple[_T_co, ...]) -> bool: ...
def __ne__(self, x: Tuple[_T_co, ...]) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

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

This is also not strictly true -- you can write (1,) == 0 and it'll just return False.

However, there's discussion in python/mypy#1271, so I'm not sure what to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this issue affects other type checkers as well, not just mypy, I created a typing issue where we can discuss this: python/typing#378

@vlasovskikh
Copy link
Member Author

The attributes of object is a special case for a static analysis tool. Both __slots__ and __dict__ are missing in object() instances, but either one or the other is present in the inheritors of object.

I've added __dict__ by analogy to __slots__ that already was here in the stub for object.

Since object is really a special case, we in PyCharm can live without __slots__ and __dict__ in the stub. In that case we will put the knowledge about these attributes into our analysis engine.

Lets either keep or remove both __slots__ and __dict__ (and add special handling for them in our type checkers in the case of removal if needed). @gvanrossum What do you think?

@vlasovskikh
Copy link
Member Author

I would try to keep typeshed universally applicable to all the type checkers. At least for now known incompatibility issues seem not that severe to me and I'm looking forward to make our code analysis in respect to understanding typeshed more compatible with PEP 484 and other type checkers. I hope the same applies to other contributors.

@JukkaL
Copy link
Contributor

JukkaL commented Jan 30, 2017

The intention of typeshed is to be useful for all Python type checkers. It's currently somewhat biased towards mypy and pytype, since they have used typeshed for the longest time, but we are happy to discuss the removal of mypyisms etc. These may take a while to be implemented, since they may require changes to mypy first. If we ask you to create a separate issue for something, it doesn't mean that we don't want that change. It may mean that we have to modify mypy before we can merge a part of a PR, since we also want to keep typeshed compatible with tools that already use it.

@vlasovskikh
Copy link
Member Author

@JukkaL Yes, sure. I was answering to Guido's comment above about the possibility of adding analyzer-specific checks similar to sys.version_info and sys.platform.

@gvanrossum
Copy link
Member

For cases where the fix for mypy is difficult or debatable, we could implement a workaround in typeshed using if MYPY -- this works today, as follows:

MYPY = False
if MYPY:
    <version for mypy>
else:
    <version for other type-checkers>

Assuming that other type-checkers treat if MYPY as if False. We should add a comment pointing to the mypy issue that needs to be fixed before the mypy-specific code can be removed.

@ambv
Copy link
Contributor

ambv commented Jan 30, 2017

@vlasovskikh If we start sprinkling typeshed with if MYPY:, please contribute some tests we can run against your code. I'm afraid to leave the "non-MYPY"-specific branches in Python 3 type stubs untested. I'd like PyCharm to be more of a first-class citizen in terms of typeshed usage.

@vlasovskikh
Copy link
Member Author

Lets at least try to continue without specific checks for type checkers. As we at PyCharm are gradually switching to typeshed we keep finding new PEP 484 incompatibilities in our checker. And we would prefer to fix them rather then add PyCharm-specific checks in stubs.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 30, 2017 via email

@ambv
Copy link
Contributor

ambv commented Jan 30, 2017

Agreed, the less checker-specific code, the better. Andrey, maybe there's an easy way for you to provide us with some small checker that we could integrate as part of runtests.sh and Travis CI? That would help ensuring we don't introduce any problems in stubs for PyCharm in the future.

@vlasovskikh
Copy link
Member Author

@gvanrossum Yes, I'm OK with that. We'll keep our own set of patches for the open PRs for a while.

@vlasovskikh
Copy link
Member Author

vlasovskikh commented Jan 31, 2017

@ambv Good idea! We're setting up internal tests for typeshed stubs for PyCharm. I hope we will make them runnable as a part of runtests.sh eventually.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Again, a suggestion to move forward with the non-controversial changes separately.


def __init__(self) -> None: ...
def __new__(cls) -> Any: ...
def __setattr__(self, name: str, value: Any) -> None: ...
def __eq__(self, o: object) -> bool: ...
def __ne__(self, o: object) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

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

Also, while the methods may not exist, the operations from which they are typically used (== and !=) do work on objects.

@vlasovskikh
Copy link
Member Author

@gvanrossum Agree, I'll re-send non-controversial bits of this request as another PR.

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.

4 participants