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-99284: [ctypes] remove _use_broken_old_ctypes_structure_semantics_ #99285

Merged
merged 4 commits into from
Nov 19, 2022

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 9, 2022

@sobolevn sobolevn changed the title Issue 99284 gh-99275: [ctypes] remove _use_broken_old_ctypes_structure_semantics_ Nov 9, 2022
@sobolevn sobolevn changed the title gh-99275: [ctypes] remove _use_broken_old_ctypes_structure_semantics_ gh-99284: [ctypes] remove _use_broken_old_ctypes_structure_semantics_ Nov 9, 2022
@sobolevn
Copy link
Member Author

sobolevn commented Nov 9, 2022

I've messed up the issue number, sorry!

@gpshead
Copy link
Member

gpshead commented Nov 13, 2022

Looks like this hack was added in ~2006. Some research of existing code to see if anything still relies on it is warranted before merging, it seems very unlikely. if so we'd want a deprecation period.

@gpshead gpshead self-assigned this Nov 13, 2022
@sobolevn
Copy link
Member Author

Quoting the original issue:

I've never seen this before, so I went and searched for this on the internet: https://cs.github.com/?scopeName=All+repos&scope=&q=_use_broken_old_ctypes_structure_semantics_

I've done a minimal research. I was not able to find any references to it.
Even Google does not have any significant references: https://www.google.com/search?newwindow=1&q=_use_broken_old_ctypes_structure_semantics_&nfpr=1&sa=X&ved=2ahUKEwiXxoHT_av7AhUQuIsKHcpYAfAQvgUoAXoECAUQAg&biw=1280&bih=726&dpr=2

I don't feel qualified in this specific module and its history to make an educated decision here. However, deprecation period is always a good idea 👍

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM, but please complete the doc.

This attribute was untested and undocumented. I never heard about it.

Looks like not a single GitHub project uses it.

I ran a code search in PyPI top 5000 projects: no projects use it.

$ ./search_pypi_top.py PYPI-2020-09-01/ '_use_broken_old_ctypes_structure_semantics_' -q

Found 0 matching lines in 0 projects

Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

Thanks! FWIW I also searched our huge internal codebase at Google, nothing found that uses this odd thing.

@gpshead gpshead merged commit a3360fa into python:main Nov 19, 2022
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