Skip to content

Rename method/member types #4927

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

Merged
merged 2 commits into from
Apr 25, 2023
Merged

Conversation

youknowone
Copy link
Member

  • PyMemberDef: Matches to CPython name and similar naming as PyMethodDef. PyMehtodDef is a python object in CPython
  • PyMemberDescrObject -> PyMemberDescriptor
    • we don't use -Object suffix
    • we avoid abbr as much as possible

@youknowone
Copy link
Member Author

cc @moreal

@youknowone youknowone requested a review from fanninpm April 24, 2023 19:56
@@ -5,7 +5,7 @@ use super::{
use crate::{
builtins::{
descriptor::{
DescrObject, MemberDef, MemberDescrObject, MemberGetter, MemberKind, MemberSetter,
DescrObject, MemberGetter, MemberKind, MemberSetter, PyMemberDef, PyMemberDescriptor,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to also have PyMemberGetter, PyMemberKind, and PyMemberSetter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Though I am not confident of this logic,

Those types are RustPython's internal while PyMemberDef is a part of public python C-API starting with PyMember_

We don't use Py prefix for our internal but use it for exposed Python types. But not sure about the exact boundary yet.

Copy link
Member

@DimitrisJim DimitrisJim left a comment

Choose a reason for hiding this comment

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

lgtm. do we have more naming refactoring we need to do? We should definitely document the naming convention someplace to have it as a point of reference.

@DimitrisJim DimitrisJim merged commit d9b6585 into RustPython:main Apr 25, 2023
@youknowone youknowone deleted the rename-types branch April 25, 2023 06:01
@youknowone
Copy link
Member Author

Maybe we'd better to study more on how pyo3 names them.

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.

3 participants