Skip to content

bpo-26243: data= is positional-only just on CPython#11754

Closed
ambv wants to merge 1 commit intopython:mainfrom
ambv:cpython_only_zlib
Closed

bpo-26243: data= is positional-only just on CPython#11754
ambv wants to merge 1 commit intopython:mainfrom
ambv:cpython_only_zlib

Conversation

@ambv
Copy link
Copy Markdown
Contributor

@ambv ambv commented Feb 4, 2019

PyPy can take it as keyword just fine.

https://bugs.python.org/issue26243

PyPy can take it as keyword just fine.
@serhiy-storchaka
Copy link
Copy Markdown
Member

I think it would be more correct to remove too strong failing assertions. Other tests should be passed on PyPy.

mozillazg pushed a commit to mozillazg/pypy that referenced this pull request Feb 4, 2019
Copy link
Copy Markdown
Contributor

@eamanu eamanu left a comment

Choose a reason for hiding this comment

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

LGTM. I test it on ubuntu 18.04 and its good.

Comment thread Lib/test/test_zlib.py
x = zlib.compress(HAMLET_SCENE)
self.assertEqual(zlib.decompress(x), HAMLET_SCENE)

@support.cpython_only
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's only the TypeError check that should be skipped, right? Not the whole test.

Comment thread Lib/test/test_zlib.py
self.assertIsInstance(dco.unconsumed_tail, bytes)
self.assertIsInstance(dco.unused_data, bytes)

@support.cpython_only
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

@csabella
Copy link
Copy Markdown
Contributor

csabella commented Jun 5, 2019

@ambv, please check @pitrou's review. Thanks!

@ambv ambv closed this Jul 12, 2021
@ambv ambv deleted the cpython_only_zlib branch July 12, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants