Repository navigation
Document removal criteria for pytest warning filters - #6595
Merged
Merged
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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
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.
This PR documents, next to every pytest
filterwarningsentry inpyproject.toml, the condition under which that entry can be removed, and refreshes the upstream status notes that had become stale.Motivation
The tracking issue of each filter states its action item as removing the entry "once upstream ships the fix". For a dependency we require without a floor, that criterion is wrong: an upstream release does not stop older releases from being supported configurations, and
filterwarningsis part of the test configuration we declare for all of them, not a snapshot of what CI happens to resolve today. Removing an entry when the fix is merely released brings the warning noise back for every supported older version, including for contributors running the suite in an existing environment.The ambiguity is not theoretical:
_check_is_sizewarning filter #6590 proposed dropping the_check_is_sizefilter as soon asbitsandbytes0.50.0 reached CI, faithfully implementing the criterion as worded in Remove bitsandbytes_check_is_sizewarning filter once the minimum required bitsandbytes is >= 0.50.0 #6447.torch.jit.scriptfilters although theirdeepspeedfix has been released since 0.18.6, precisely because our floor has not moved yet.So both readings currently coexist in the same file, with nothing on the entries themselves to tell them apart. Recording the criterion at each entry puts that information where the removal decision is actually taken, rather than in an issue title.
Solution
Add one
Remove once:line per entry. The criterion takes one of two shapes, depending on whether TRL can constrain the component that emits the warning:bitsandbytes,liger-kernel,deepspeed), removal is gated on our minimum required version reaching the first release that contains the fix;triton, pinned bytorch) or resolved at runtime (thekernels-community/mamba-ssmHub kernel build), there is no floor to bump, so removal is gated on no supported configuration emitting the warning any more.The criterion lines deliberately do not restate the version we currently require, which would be one more thing to keep in sync with the requirements.
Reviewing every entry also surfaced three upstream status notes that are no longer accurate, updated here. The wording of the tracking issues (#6435, #6447, #6466, #6555) is aligned separately.
Changes
Remove once:line to everyfilterwarningsentry, giving the target version for the entries gated on a requirement bump and the observable condition for the othersliger-kernelSAPO note: the fix is released in 0.8.1, which we exclude for an unrelated crash (Hotfix CI: Temporarily pin liger-kernel < 0.8.1 #6517), so the criterion is 0.8.2bitsandbytes_check_is_sizenote: the fix is released in 0.50.0tritonAnnAssignentry, merged and unreleased as of 3.7.1Note
Low Risk
Comment-only changes to test configuration documentation; no runtime or dependency requirement changes.
Overview
Adds a
Remove once:comment beside each pytestfilterwarningsentry inpyproject.toml, so maintainers know when to drop a filter without relying on tracking-issue wording alone.Criteria differ by dependency: filters tied to declared extras (e.g. deepspeed, liger-kernel, bitsandbytes) say removal waits until TRL’s minimum required version includes the upstream fix; others (sentencepiece, torch, triton, Hub mamba kernel) say removal waits until no supported configuration still emits the warning.
Also refreshes stale upstream notes: liger-kernel SAPO fix is in 0.8.1 with removal gated on >= 0.8.2 (0.8.1 excluded per GH-6517), bitsandbytes
_check_is_sizefix is in 0.50.0, and tritonAnnAssigndocuments merged fix PR #10986 (unreleased as of 3.7.1).Reviewed by Cursor Bugbot for commit 83fcf57. Bugbot is set up for automated code reviews on this repo. Configure here.