Skip to content

switching _pytest.code to new style classes broke the api #2398

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
RonnyPfannschmidt opened this issue May 10, 2017 · 6 comments
Closed

switching _pytest.code to new style classes broke the api #2398

RonnyPfannschmidt opened this issue May 10, 2017 · 6 comments
Labels
status: critical grave problem or usability issue that affects lots of users type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: regression indicates a problem that was introduced in a release which was working previously

Comments

@RonnyPfannschmidt
Copy link
Member

@nicoddemus @hpk42 this one is fun

due to

>>> class a:
...  pass
... 
>>> a()[1]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: a instance has no attribute '__getitem__'
>>> class b(object): pass
... 
>>> b()[1]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'b' object does not support indexing
>>> 

we changed the behaviour of feature detection on report.long_repr

@RonnyPfannschmidt RonnyPfannschmidt added type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog status: critical grave problem or usability issue that affects lots of users type: regression indicates a problem that was introduced in a release which was working previously labels May 10, 2017
@The-Compiler
Copy link
Member

Which class is affected by this? Here's a crazy idea: What about adding a __getitem__ to that class which raises an AttributeError (or a custom error which subclasses both AttributeError and TypeError)?

@nicoddemus
Copy link
Member

Which class is affected by this?

IIRC originally it was TerminalRepr which was changed in fbc5ba0, but since we changed all class declarations to subclass from object explicitly in #2179, this affects all classes now.

It is important to note that the purpose of #2179 was to exactly remove the subtle differences between old and new classes in Python 2 because pytest wasn't consistent regarding this with some classes subclassing object and others not.

Here's a crazy idea: What about adding a getitem to that class which raises an AttributeError (or a custom error which subclasses both AttributeError and TypeError)?

I agree with the "crazy" part of it. 😛 I would rather not make things worse by adding black magic just to fix this, hehehe.

@RonnyPfannschmidt
Copy link
Member Author

@The-Compiler its next to impossible to ensure we didn't mis some random painful edge-cases, i'd prefer admitting an accidental break so people can review and fix their stuff

1 similar comment
@RonnyPfannschmidt
Copy link
Member Author

@The-Compiler its next to impossible to ensure we didn't mis some random painful edge-cases, i'd prefer admitting an accidental break so people can review and fix their stuff

@RonnyPfannschmidt
Copy link
Member Author

@The-Compiler its next to impossible to ensure we didn't miss some random painful edge-cases, i'd prefer admitting an accidental break so people can review and fix their stuff

nicoddemus added a commit to nicoddemus/pytest that referenced this issue May 17, 2017
As discussed in the mailing list, unfortunately this might break APIs
due to the subtle differences between new and old-style classes (see pytest-dev#2398).

This reverts commit d4afa15 from PR pytest-dev#2179.
@nicoddemus
Copy link
Member

Can we close this given that #2414 has been merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: critical grave problem or usability issue that affects lots of users type: backward compatibility might present some backward compatibility issues which should be carefully noted in the changelog type: regression indicates a problem that was introduced in a release which was working previously
Projects
None yet
Development

No branches or pull requests

3 participants