Polygraphy - fix POLYGRAPHY_ASK_BEFORE_INSTALL=0 still enabling the install prompt - #4836
Open
VaggelisGian wants to merge 2 commits into
Open
Polygraphy - fix POLYGRAPHY_ASK_BEFORE_INSTALL=0 still enabling the install prompt#4836VaggelisGian wants to merge 2 commits into
VaggelisGian wants to merge 2 commits into
Conversation
The default value passed to os.environ.get was the boolean expression
"0" != "0" instead of the string "0", so the lookup could never return
a falsy default and bool("0") is True. Setting POLYGRAPHY_ASK_BEFORE_INSTALL=0
thereby enabled the prompt it was meant to disable. Use the same
get(KEY, "0") != "0" idiom as the neighboring flags, and cover all three
cases (unset, 0, 1) with a regression test that reloads the config module.
Test Plan:
cd tools/Polygraphy
PYTHONPATH=$PWD python -m pytest tests/test_config.py -v
fails before the fix on the =0 case (assert not True), passes after
PYTHONPATH=$PWD python -m pytest tests/test_config.py tests/comparator tests/common -q
identical failure set to the suites alone (all pre-existing environment
failures), proving no reload side effects or ordering dependence
Signed-off-by: Vaggelis <baggelis100@gmail.com>
Upstream appends entries for unreleased changes to the existing undated version section rather than opening a new heading. Signed-off-by: Vaggelis <baggelis100@gmail.com>
| """ | ||
|
|
||
| ASK_BEFORE_INSTALL = bool(os.environ.get("POLYGRAPHY_ASK_BEFORE_INSTALL", "0" != "0")) | ||
| ASK_BEFORE_INSTALL = bool(os.environ.get("POLYGRAPHY_ASK_BEFORE_INSTALL", "0") != "0") |
Collaborator
There was a problem hiding this comment.
This is actually already fixed internally and would go out with the next release, but thanks for fixing it here!
pranavm-nvidia
approved these changes
Aug 24, 2026
Author
|
Thanks for the quick review and approval! Good to know it is also fixed internally for the next release - if you would rather take that one and close this PR, that works fine on my end. Either way is fine. |
Collaborator
@VaggelisGian the fix is the same, but I like your test better. We can merge your PR and I'll integrate it with the internal changes. |
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
POLYGRAPHY_ASK_BEFORE_INSTALLcould not be disabled. Intools/Polygraphy/polygraphy/config.py, the flag was parsed as:The default argument is the boolean expression
"0" != "0"(evaluates toFalse), instead of the string"0". Becausebool("0")isTrue, any value present in the environment - including0- enabled the install prompt, and there was no way to turn it off.The fix moves the comparison outside the call, matching the idiom used by the sibling flags in the same file:
Testing
New test module
tests/test_config.py::TestConfigEnvVars::test_ask_before_install, covering unset,0and1values viaimportlib.reloadwith environment restore.Commands and output on this machine (after the fix):
Before the fix the test fails: with
POLYGRAPHY_ASK_BEFORE_INSTALL=0in the environment the reloaded config reportsASK_BEFORE_INSTALL == True. The single warning in the output (Marks applied to fixtures have no effect, fromtests/conftest.py) is pre-existing on main and unrelated.Environment
TensorRT OSS main (10d15ae), polygraphy sources run from the repo.
Python 3.12, tensorrt 11.2.1.2 (pip wheel), polygraphy 0.53.4,
NVIDIA RTX 5060 Ti, driver 591.86, Windows 11 Pro.
Issue
Per CONTRIBUTING.md, bugfixes start as an issue approved by TensorRT engineers before code review. I could not find an existing issue covering this; happy to file one and link it here if maintainers prefer that flow, or please advise whether this PR can proceed directly.