cpython icon indicating copy to clipboard operation
cpython copied to clipboard

gh-98718: re-use already calculated is_python_build from getpath

Open FFY00 opened this issue 3 years ago • 6 comments

This also gets rid of the duplicated logic.

  • Issue: gh-98718

FFY00 avatar Oct 26 '22 13:10 FFY00

Seems fine to me, though I assume some others have more opinions about tacking stuff onto the sys module. I think it's fine.

zooba avatar Oct 26 '22 20:10 zooba

I think the biggest win here is really unifying the logic in one place.

FFY00 avatar Oct 26 '22 21:10 FFY00

Does not it need documentation?

Would not it be more useful to expose build_prefix (None or not set if is not Python ) rather of simple boolean _is_python_build?

serhiy-storchaka avatar Oct 27 '22 13:10 serhiy-storchaka

Does not it need documentation?

As proposed, I see it as an internal implementation detail, so wouldn't be documented.

Would not it be more useful to expose build_prefix (None or not set if is not Python ) rather of simple boolean _is_python_build?

+1. is_python_build() could still return bool(sys._build_prefix).

jaraco avatar Nov 06 '22 21:11 jaraco

Is sys._is_python_build always a correct indicator at runtime?

Nope, but we don't have a proper one. Any marker file could disappear or be copied, so we may as well look for the things we actually care about (stdlib modules) and guess, rather than look for something that's there solely to help us guess.

At least if we expose it properly, it'll be easier to change it to something more reliable if we come up with one.

zooba avatar Nov 07 '22 20:11 zooba

Does not it need documentation?

It's missing a news entry, but other than that, no, it's an internal change.

Would not it be more useful to expose build_prefix (None or not set if is not Python ) rather of simple boolean _is_python_build?

We can do that. The reason I didn't do it here was because I didn't want to add any more public API, so I just used the already exported _is_build_prefix. I opened #99273 following this suggestion.

FFY00 avatar Nov 09 '22 03:11 FFY00