FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add OSError.__reduce__ by dvermd · Pull Request #4191 · RustPython/RustPython · GitHub

Repository navigation

Add OSError.__reduce__ - #4191

Merged
youknowone merged 1 commit into
RustPython:mainfrom
dvermd:oserror_reduce
Oct 4, 2022
Merged

youknowone merged 1 commit into
RustPython:mainfrom
dvermd:oserror_reduce

Conversation

dvermd commented Oct 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 marked this pull request as draft October 2, 2022 06:28
dvermd force-pushed the oserror_reduce branch 3 times, most recently from a00dac2 to 637870a Compare October 2, 2022 21:52
dvermd marked this pull request as ready for review October 2, 2022 22:07
Comment thread vm/src/exceptions.rs Outdated
// 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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
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 thread vm/src/exceptions.rs Outdated
Comment on lines +1347 to +1357
if new_args.args.len() >= 2 {
new_args.args.truncate(2);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

dvermd mentioned this pull request Oct 3, 2022
dvermd force-pushed the oserror_reduce branch 3 times, most recently from df5bca4 to e7b6f1e Compare October 4, 2022 11:35
youknowone merged commit 3a36e55 into RustPython:main Oct 4, 2022
dvermd deleted the oserror_reduce branch October 10, 2022 21:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL