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

Rename PyMethodsImpl -> PyMethods #951

Merged
merged 1 commit into from
Jun 3, 2020
Merged

Rename PyMethodsImpl -> PyMethods #951

merged 1 commit into from
Jun 3, 2020

Conversation

kngwyu
Copy link
Member

@kngwyu kngwyu commented Jun 3, 2020

As I'll introduce PyProtoMethods, I renamed PyMethodsImple for consistent naming.

@kngwyu
Copy link
Member Author

kngwyu commented Jun 3, 2020

Actions says


running 1 test
ERROR: failed to read cargo metadata: EOF while parsing a value at line 1 column 0

Wow.

@davidhewitt
Copy link
Member

How about just having one trait maybe called PyClassMembers with py_methods() and py_proto_methods()? Maybe this isn't possible if PyProtoMethods won't always be implemented.

I don't have strong opinion though so please continue with two traits if you prefer to keep them separate!

@kngwyu
Copy link
Member Author

kngwyu commented Jun 3, 2020

just having one trait maybe called PyClassMembers

I think it's possible but can be confusing since PyProtoMethods has many methods like:

pub trait PyProtoMethods {
    fn basic_methods() -> Option<NonNull<PyObjectMethods>> {
        None
    }
    fn buffer_methods() -> Option<NonNull<PyBufferProcs>> {
        None
    }
    fn descr_methods() -> Option<NonNull<PyDescrMethods>> {
        None
    }
...
}

@kngwyu kngwyu merged commit 75b2b62 into master Jun 3, 2020
@messense messense deleted the pymethod-refactor branch March 18, 2021 02:23
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.

2 participants