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

Add OSError.__reduce__ #4191

Merged
merged 1 commit into from Oct 4, 2022
Merged

Add OSError.__reduce__ #4191

merged 1 commit into from Oct 4, 2022

Conversation

dvermd
Copy link
Contributor

@dvermd dvermd commented Oct 1, 2022

Tested by removing expected failure and commenting everything but OSError in Lib/test/test_exceptions.py::testAttributes::exceptionList, WindowsError of the test.

Part of #3611

@dvermd dvermd marked this pull request as draft October 2, 2022 06:28
@dvermd dvermd force-pushed the oserror_reduce branch 3 times, most recently from a00dac2 to 637870a Compare October 2, 2022 21:52
@dvermd dvermd marked this pull request as ready for review October 2, 2022 22:07
@@ -764,6 +765,7 @@ impl ExceptionZoo {
// second exception filename
"filename2" => ctx.none(),
"__str__" => ctx.new_method("__str__", excs.os_error, os_error_str),
"__reduce__" => ctx.new_method("__str__", excs.os_error, os_error_reduce),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"__reduce__" => ctx.new_method("__str__", excs.os_error, os_error_reduce),
"__reduce__" => ctx.new_method("__reduce__", excs.os_error, os_error_reduce),

Comment on lines 1347 to 1357
if new_args.args.len() >= 2 {
new_args.args.truncate(2);
}
Copy link
Member

Choose a reason for hiding this comment

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

This is a question because I don't know the spec very well. Exceptions usually takes infinite args and store it. What happens for more args than 4 in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to work that no matter how we put arguments to its parenthesis (work means it does not throw exceptions).
But according to code and behavior. it works as expected when only the given argument length is between 2 and 5.

>>> OSError(1,2).errno
1
>>> OSError(1,2).strerror
2
>>> OSError(1)
OSError(1)
>>> OSError(1).errno

https://github.com/python/cpython/blob/3.10/Objects/exceptions.c#L820-L876

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss this point in #4194 ...

@dvermd dvermd mentioned this pull request Oct 3, 2022
@dvermd dvermd force-pushed the oserror_reduce branch 3 times, most recently from df5bca4 to e7b6f1e Compare October 4, 2022 11:35
@youknowone youknowone merged commit 3a36e55 into RustPython:main Oct 4, 2022
@dvermd dvermd deleted the oserror_reduce branch October 10, 2022 21:09
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

3 participants