-
Notifications
You must be signed in to change notification settings - Fork 790
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
Combine sys_platform
and platform_system
in marker algebra
#9344
base: charlie/lower-ii
Are you sure you want to change the base?
Conversation
cfc5550
to
1c03cb9
Compare
Yes. I was just about to comment on #9275 about this. For example, |
Yeah. I think it's a bit more challenging because there isn't a 1-to-1 mapping between |
(Not impossible but different than what I've encoded here.) |
Since |
51ecdd7
to
0340fb3
Compare
4b8f5c7
to
d51b0b5
Compare
On reflection, what I have here is not correct. It fails this test (i.e., it thinks these are disjoint): assert!(!is_disjoint(
"sys_platform == 'cygwin'",
"platform_system == 'Java'"
)); |
Somewhat losing confidence here because it turns out Android also returns |
0340fb3
to
ae3be00
Compare
d51b0b5
to
b018724
Compare
It seems like Poetry has gone though quite the rabbit hole for this themselves: python-poetry/poetry#5192. But even after quite some pull requests, they closed the isue with this comment:
It appears this may be an unresolvable issue, though perhaps it's feasible to just cull known impossible environments? I.e. we know that |
Summary
This PR adds a representation to the marker algebra to capture "known pairs" of
sys_platform
andplatform_system
values.For example, we want to be able to detect that
sys_platform == 'win32'
andplatform_system == 'Darwin'
are disjoint.There is some risk here, in that if there are platforms for which, e.g.,
sys_platform == 'win32
butplatform_system
is notWindows
, we'll get things wrong. I'm trying to weigh whether that's a real risk.In theory, we could maybe also include
os.name
here?Closes #7760.
Closes #9275.