Skip to content

Count only the cost warning in the coordinate_patch_hooks warn-once test - #1910

Open
YHC66 wants to merge 2 commits into
TransformerLensOrg:devfrom
YHC66:fix-coordinate-patch-warning-count-newer-beartype
Open

YHC66 wants to merge 2 commits into
TransformerLensOrg:devfrom
YHC66:fix-coordinate-patch-warning-count-newer-beartype

Conversation

@YHC66

@YHC66 YHC66 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Description

test_coordinate_patch_hooks_warns_once_naming_layer_and_position_counts fails on dev (177d2cc1) with an up-to-date beartype:

>       assert len(record) == 1
E       assert 5 == 1

Since #1907, coordinate_patch_hooks wraps an inner transform(..., positions: List[int], ...) for each layer. Under the jaxtyping import hook, beartype >= 0.22 (checked 0.22.9 and 0.23.1) emits a BeartypeDecorHintPep585DeprecationWarning for that typing.List hint, and that class is a UserWarning subclass. pytest.warns records every warning raised in its block regardless of the filterwarnings ignore in pyproject.toml, so with two layers the record holds the cost warning plus four beartype notices. CI does not see this because uv.lock pins beartype 0.14.1, but pyproject.toml allows beartype>=0.14.1, so a fresh environment gets 0.23.1.

The test now counts only the cost warning (matched on its message), which keeps the "warns once" check intact: injecting a second cost warning into coordinate_patch_hooks still makes it fail. No library code changes. If you would rather drop the beartype notices at the source, switching the inner transform hints to builtin list[int] would be the alternative; happy to do that instead.

Checked on dev 177d2cc1, macOS arm64, Python 3.12:

  • beartype 0.23.1 (transformers 5.20.1, torch 2.11.0): this test fails before the change and passes after; tests/unit/tools 1093 passed. With beartype 0.22.9, tests/unit before the change: 7461 passed, this test the only failure.
  • beartype 0.14.1 (locked env, transformers 5.13.0): tests/unit/tools 1093 passed.
  • The test passes at 413a38b9~1 and fails at 413a38b9 (Analysis Tools Review & Re-contextualization #1907) with beartype 0.23.1.
  • black and isort are clean on the changed file.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have not rewritten tests relating to key interfaces which would affect backward compatibility

pytest.warns records every warning raised in its block. Since TransformerLensOrg#1907 the
coordinate-patch hooks wrap an inner transform annotated with typing.List,
and beartype >= 0.22 emits a BeartypeDecorHintPep585DeprecationWarning
(a UserWarning subclass) for it under the jaxtyping import hook, so the
test saw 5 warnings instead of 1 with an up-to-date beartype. The global
filterwarnings ignore does not apply inside pytest.warns.

@jlarson4 jlarson4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment on inline commenting consistency

with pytest.warns(UserWarning, match=r"2 layer\(s\) x 2 position\(s\)") as record:
lens.coordinate_patch_hooks(toy_model, 3, 5, layers=[0, 1], positions=[0, 1])
assert len(record) == 1
# pytest.warns records every warning raised in the block, including beartype's PEP 585

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please trim this comment down to a single line if possible?

Providing this level of detail about the specific bear type issue that prompted this fix is asking for the comment to go stale. Once the UserWarning described here is fully deprecated and removed from bear type, the comment is referencing something that no longer exists.

This comment could simply be the following:

# Ensure test only fails due to the warning we are testing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, trimmed it to your suggested one-liner in b3b8938.

@koriyoshi2041

Copy link
Copy Markdown
Contributor

Checked exact head b3b89387. The focused coordinate-patch hook file passes 12/12 locally, and the assertion remains scoped to the actual cost-warning text while unrelated UserWarning subclasses can coexist in the capture block. The complete hosted compatibility, type, format, coverage, benchmark, and notebook matrix is green; git diff --check is clean.

This branch has not been deployed

No deployments
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.

3 participants