Skip to content

Comments

use Initiailizer for PyStaticMethod#3742

Merged
youknowone merged 1 commit intoRustPython:mainfrom
HyunP-dev:staticmethod
Aug 7, 2022
Merged

use Initiailizer for PyStaticMethod#3742
youknowone merged 1 commit intoRustPython:mainfrom
HyunP-dev:staticmethod

Conversation

@HyunP-dev
Copy link
Contributor

@HyunP-dev HyunP-dev commented May 22, 2022

No description provided.

youknowone
youknowone requested changes May 22, 2022
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

running cargo fmt --all will fix the fmt failure

youknowone
youknowone requested changes May 22, 2022
View reviewed changes
vm/src/builtins/staticmethod.rs Outdated
fn init(zelf: PyRef<Self>, args: Self::Args, vm: &VirtualMachine) -> PyResult<()> {
zelf.as_object()
.set_attr("__doc__", args.get_attr("__doc__", vm)?, vm)?;
// zelf.as_object().set_attr("__format__", args.get_attr("__format__", vm)?, vm);
Copy link
Member

@youknowone youknowone May 22, 2022

Choose a reason for hiding this comment

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

Suggested change
// zelf.as_object().set_attr("__format__", args.get_attr("__format__", vm)?, vm);

@Snowapril
Copy link
Contributor

Snowapril commented May 22, 2022

expectedFailure above the test_decorators::test_staticmethod need to be removed.

And it seems test_reprlib::test_descriptors codes are outdated and it causes the FAIL.
You need to update Lib/test/test_reprlib.py up to python 3.10 (ref https://github.com/python/cpython/blob/3.10/Lib/test/test_reprlib.py#L195-L208)
Or add expectedFailure on that test and add separate PR for updating it.

@fanninpm fanninpm mentioned this pull request May 26, 2022
Merged
@fanninpm
Copy link
Contributor

fanninpm commented May 26, 2022

Or add expectedFailure on that test and add separate PR for updating it.

Now that #3747 has been merged, can this be rebased on a fresh copy of the main branch?

@youknowone youknowone mentioned this pull request Jun 15, 2022
Merged
@youknowone youknowone changed the title Update staticmethod.rs use Initiailizer for PyStaticMethod Aug 7, 2022
youknowone
youknowone reviewed Aug 7, 2022
View reviewed changes
vm/src/builtins/staticmethod.rs Outdated
Comment on lines 80 to 81
zelf.as_object()
.set_attr("__doc__", args.get_attr("__doc__", vm)?, vm)?;
Copy link
Member

@youknowone youknowone Aug 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

this is not a fix. func object need to be replaced, not doc.

youknowone
youknowone approved these changes Aug 7, 2022
View reviewed changes
Copy link
Member

@youknowone youknowone left a comment

Choose a reason for hiding this comment

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

no behavior changed but harmless and preparation for next step

@youknowone youknowone merged commit d39a28b into RustPython:main Aug 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HyunP-dev @Snowapril @fanninpm @youknowone