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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants