-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add framework attribute in sys module #1343
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for contributing!
vm/src/sysmodule.rs
Outdated
@@ -225,6 +225,12 @@ pub fn make_module(vm: &VirtualMachine, module: PyObjectRef, builtins: PyObjectR | |||
"unknown".to_string() | |||
}; | |||
|
|||
let framework = if cfg!(target_of = "macos") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let framework = if cfg!(target_of = "macos") { | |
let framework = if cfg!(target_os = "macos") { |
Thanks for the contribution. Here are a few concerns about this patch (and the issue too).
|
Thank you for comment. Currently this code only affects the Lines 257 to 259 in 1dceae9
Lines 271 to 272 in 1dceae9
CPython also uses the
This function works as follows on my system:
As you commented, this value seems to be an empty string. In such a case, the following value will always be returned. And this attribute is defined on all platforms. However, it is only implemented in 3.7+ and up, so it cannot be used in lower versions. For example, in my windows(anaconda 3.7.3) environment it would run:
In a Mac(use pyenv 3.6.6) environment, it runs like this:
Thanks for reading! |
8b76919
to
0fd06a0
Compare
Thanks for detailed explanation. I am sorry to confuse it. It seems CI fails because RustPython CI now tests only with CPython 3.6. |
I apologize for writing the wrong test case first. For RustPython developers using mac, this test may fail even with Python 3.7+. Actually this test fails on my Mac. This is because the system python on Mac is built with
I thought about testing in the There are a few things to think about for this test.
The best code I can think of is this:
What do you think about this solution? |
0fd06a0
to
f13f772
Compare
now, actually, the code itself looks totally fine. but not sure the test code is fit for the goal of snippet tests. |
f13f772
to
1be7cc6
Compare
Fixes #1342