Skip to content

gh-142254: add @cpython_only to tests relying on deepcopy implementation#142324

Closed
Bobronium wants to merge 1 commit intopython:mainfrom
Bobronium:mark-deepcopy-memo-tests-cpython-only
Closed

gh-142254: add @cpython_only to tests relying on deepcopy implementation#142324
Bobronium wants to merge 1 commit intopython:mainfrom
Bobronium:mark-deepcopy-memo-tests-cpython-only

Conversation

@Bobronium
Copy link
Copy Markdown
Contributor

@Bobronium Bobronium commented Dec 5, 2025

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Dec 7, 2025

@serhiy-storchaka What do you think of this one? strictly speaking, we're reducing test coverage of other implementations but I don't know if @cpython_only applies to stdlib in pure Python. Strictly speaking, our pickle implementation deviates from the docs themselves so it's strictly speaking, a cpython-only thing, but I'm not sure we're really consistent actually. I'm also fine with not changing anything.

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

I do not think this is an appropriate use of cpython_only. The implementation is not CPython specific.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Dec 7, 2025

So for you cpython_only only applies to the interpreter's implemenation and not the modules in Lib right? I'm actually fine with marking it as cpython_only but I don't like the fact that other implementations that use the same code would then have a smaller test coverage. I think it's then better to not mark it as an implementation detail even if we disagree with the docs a bit, but and others will need to write their own tests instead (but I don't think we broke any major implementation right?) WDYT?

@serhiy-storchaka
Copy link
Copy Markdown
Member

If there is a major implementation which fails tests for good reason, they can be marked as CPython-only. It can be a code in Lib if it depends on CPython-specific behavior or modules.

But this is not the case.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Dec 8, 2025

Ok, let's close this one. Sorry @Bobronium but there isn't enough reason to change our current code for that.

@picnixz picnixz closed this Dec 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants