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

gh-99482: remove jython compatibility parts from stdlib and tests #99484

Merged
merged 4 commits into from Dec 23, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 14, 2022

I am marking this a draft for now, because we still wait for Jython's team response.
But, it is always easier to make decisions when you see the code :)

@@ -321,7 +321,7 @@ def raises_oserror(*a):

def test_java_ver(self):
res = platform.java_ver()
Copy link
Member Author

@sobolevn sobolevn Nov 14, 2022

Choose a reason for hiding this comment

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

I am not touching platform.java_ver(), because this is a user-facing API.

def __repr__(self):
return b'byte-repr'
self.assertRaises(TypeError, repr, WrongRepr())
# Test basic sanity of repr()
Copy link
Member Author

@sobolevn sobolevn Nov 14, 2022

Choose a reason for hiding this comment

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

These diffs are ugly, I can do if True: instead with a comment.

@@ -360,10 +360,9 @@ def test_capi3():
self.assertRaises(SystemError, _testcapi.raise_exception,
InvalidException, 1)

if not sys.platform.startswith('java'):
Copy link
Member Author

@sobolevn sobolevn Nov 14, 2022

Choose a reason for hiding this comment

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

This is not required, because there's @cpython_only decorator.

pyc = TESTFN + "$py.class"
else:
pyc = TESTFN + ".pyc"
pyc = TESTFN + ".pyc"
Copy link
Member Author

@sobolevn sobolevn Nov 14, 2022

Choose a reason for hiding this comment

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

I am removing Jython compat from tests, because there are lots and lots of new tests since 2.7

New syntax, new corner cases, etc.

If Jython ever run our new suite, there will be a lot of things to fix / adapt on their side.

Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@hauntsaninja hauntsaninja left a comment

Thanks, this looks good to me!

cc @JelleZijlstra

def __repr__(self):
return b'byte-repr'
self.assertRaises(TypeError, ascii, WrongRepr())
# Test basic sanity of repr()
Copy link
Member

@JelleZijlstra JelleZijlstra Dec 23, 2022

Choose a reason for hiding this comment

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

Not related to your PR but this comment seems wrong.

Copy link
Member Author

@sobolevn sobolevn Dec 23, 2022

Choose a reason for hiding this comment

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

I will take care of it later 👍

@hauntsaninja hauntsaninja merged commit 745545b into python:main Dec 23, 2022
16 checks passed
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.

None yet

4 participants