Skip to content

Conversation

BenjaminBossan
Copy link
Member

Low priority.

What does this PR do?

I noticed that the test should be for the option check_compiled="ignore" but it was using check_compiled="warn". This has been fixed, now the correct argument is passed.

However, the fact that the test passed means that it was incorrect to begin with. The way that logs are collected does not collect the logger.warning call here (not sure why). To amend this, I'm now using assertNoLogs. With this change, the test correctly fails when the wrong argument is passed. Note that assertNoLogs requires Python 3.10+ (3.9 is EOL since yesterday 🎉).

Before submitting

Low priority.

I noticed that the test should be for the option check_compiled="ignore"
but it was using check_compiled="warn". This has been fixed, now the
correct argument is passed.

However, the fact that the test passed means that it was incorrect to
begin with. The way that logs are collected does not collect the
logger.warning call here (not sure why). To amend this, I'm now using
assertNoLogs. With this change, the test correctly fails when the wrong
argument is passed. Note that assertNoLogs requires Python 3.10+ (3.9 is
EOL since yesterday).
@BenjaminBossan BenjaminBossan merged commit 7242b5f into huggingface:main Oct 2, 2025
24 checks passed
@BenjaminBossan BenjaminBossan deleted the fix-test-enable_lora_hotswap-ignore-warning branch October 2, 2025 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants