gh-140025: fix queue.SimpleQueue.__sizeof__() computation #140086
Conversation
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
This comment was marked as duplicate.
This comment was marked as duplicate.
…lying data structure
This comment was marked as duplicate.
This comment was marked as duplicate.
picnixz
changed the title
ZeroIntensity
left a comment
ZeroIntensity
left a comment
There was a problem hiding this comment.
Per the bot, please add a news entry. Please also avoid force-pushing; it makes things harder to review and we squash at the end anyway.
Misc/NEWS.d/next/Library/2025-10-14-14-07-08.gh-issue-140025.zQ_Fhe.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Library/2025-10-14-14-07-08.gh-issue-140025.zQ_Fhe.rst
Outdated
Show resolved
Hide resolved
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
Overall feedback:
- Implement the tests in PySimpleQueueTest and CSimpleQueueTest. They will be different becauss the impmementation is different.
- Remove messages when assertions fail and when the condition being tested is self-explanatory.
- Remove commenta for self-explanatory code.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
Please be more careful with review feedback. I also feel that this PR was AI generated because the previous version tested equality which was not asked although one of my comment mentioned it. I am at the airport and will board in a few minutes so I will be less available for the next few days. If another core dev considers the PR to be in a mergeable state, please proceed even if I didn't leave another review.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
|
The changes made are more and more unrelated and I do not understand why. As I am now going offline I will ask another core dev to review this thoroughly and check if my comments are addressed in the final version of that PR. I leave them the responsibility of removing the label as well |
|
Marking as "Awaiting changes" has comments have not been addressed. |
|
Please restore the existing tests. Also, any new tests need to have distinct names. |
exist test is not modified, only add PySimpleQueueTest and CSimpleQueueTest, i this two tests have distinct names |
|
There are clearly unrelated changes. Please look at the diff. I also highly suspect that this PR was AI-generated, or at least AI-aided. If this is not changed, I think we should close this PR. Finally, please do not ignore comments by resolving them when they are still relevant. |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Look at the sizeof tests for deque, OrderedDict and other collections.
9 participants
gh-140025 queue.SimpleQueue.sizeof() ignores the underlying data structure