Repository navigation
Implement strict_mode keyword for binascii.a2b_base64() - #4460
Conversation
strict_mode keyword for binascii.a2b_base64()
|
@DimitrisJim was wondering if you could help me out. Thanks for all your help so far! |
|
I believe you can use the following command to test your changes locally: # this runs all of the tests (not necessary if you know which test is affected)
cargo run --release --features ssl,jit -- -m test -j 1 -u all --slowest --fail-env-changed -v
# this runs only the test suite named "test_whatever" (usually located at `Lib/test/test_whatever.py`)
cargo run --release --features ssl,jit -- -m test -j 1 -u all --slowest --fail-env-changed -v test_whateverFor a list of all resources and more command-line options, you can execute |
|
Seems like As @fanninpm mentioned, you should use I'll try and make some time to check through the code to see if I can spot where any issues might be and thanks for working on it! |
|
Looks like |
|
@DimitrisJim I think (fingers crossed) this is done? |
| Ok(decoded) | ||
| }) | ||
| .map_err(|err| { | ||
| let python_error = match err { |
There was a problem hiding this comment.
impl ToPyException for base64::DecodeError is possible to replace this block
There was a problem hiding this comment.
Sorry, pretty new to Rust. Could you please provide an example in the context of the repo? Not super sure how to apply it in practice, sorry
There was a problem hiding this comment.
io.rs in vm/src/stdlib contains an implementation of ToPyException for Io::Errorwhich can definitely serve as a good example. You basically just move the match in there and use to_pyexception here.
There was a problem hiding this comment.
We have a trait called ToPyException which used to convert any error to python exception. It is used to convert Io::Error to python exception as @DimitrisJim explained.
Once you implement it, this code will look like:
.map_err(|err| err.to_pyexception(vm))If you stuck with it, please feel free to ask anything again or tell us to take over that part.
The patch is really nice and this suggestion is totally optional. You don't need to finish to implement ToPyException to finish this PR.
Thank you!
There was a problem hiding this comment.
Thanks for the kind words! I'm not sure I'll be able to get it
719d538 to
d39d708
Compare
d39d708 to
ff973ca
Compare
youknowone
left a comment
There was a problem hiding this comment.
Thank you for contributing! I appended a commit with a little of changes
New PR for #4374. Still working on figuring out how to get the error messages to match, but wanted to get this up to at least run against CI and see what else I'm missing. I think the general direction should be okay.