Skip to content

Fix negative narrowing for isinstance tuple targets#3415

Closed
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:fix-isinstance-tuple-negative-narrowing
Closed

Fix negative narrowing for isinstance tuple targets#3415
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:fix-isinstance-tuple-negative-narrowing

Conversation

@LeSingh1
Copy link
Copy Markdown
Contributor

Fixes #3412.

This fixes negative narrowing for isinstance checks where the target is a tuple of runtime classes, such as isinstance(x, (str, bytes)).

Summary

  • For isinstance(x, (A, B)), narrow_is_not_instance previously subtracted each tuple element from the full input type independently and then intersected the per-element results.
  • When the input contained a member that overlapped with multiple tuple elements (e.g. Iterable[Any] is a supertype of both str and bytes), the intersection step would reintroduce the subtracted alternatives through subset-based intersection. Concretely, intersect_impl(Iterable[Any], str) returns str because str <: Iterable[Any].
  • The fix walks the input union once and subtracts every tuple element from each union member sequentially, so the per-element scope doesn't reintroduce the overlap. Targets that don't allow negative narrowing (e.g. a non-final type[C] variable) are skipped, preserving existing conservative behavior.

Repro

from typing import reveal_type, Any, Iterable

def arrlen(obj: Any) -> int | None: ...

def main(a: float | str | bytes | Iterable[Any]) -> None:
    if isinstance(a, (str, bytes)):
        pass
    elif arrlen(a) is not None:
        reveal_type(a)

Before: Iterable[Any] | bytes | float | str
After: Iterable[Any] | float

Test plan

  • Added test_isinstance_tuple_negative_with_overlap in pyrefly/lib/test/narrow.rs.
  • cargo test -p pyrefly --lib — 5152 passed, 0 failed.
  • All 40 test_isinstance_* tests continue to pass, including the existing test_isinstance_type_negative_partial_narrow which exercises the not-final type[C] skip path.
  • Verified the repro file against the locally built target/debug/pyrefly.

Fixes facebook#3412.

For `isinstance(x, (A, B))`, pyrefly previously computed the negative
branch by subtracting each tuple element from the input type
independently and then intersecting the results. When the input
contained a type that overlapped with multiple tuple elements (e.g.
`Iterable[Any]` is iterable and is therefore a supertype of both
`str` and `bytes`), the intersection step would reintroduce the
subtracted alternatives, so

    a: float | str | bytes | Iterable[Any]
    if isinstance(a, (str, bytes)):
        ...
    else:
        reveal_type(a)  # was Iterable[Any] | bytes | float | str

came out unchanged.

The fix walks the input union once and subtracts every tuple element
from each member in turn, so members that ARE in the tuple drop out
and members that aren't (`float`, `Iterable[Any]`) are kept.

Adds a regression test in `pyrefly/lib/test/narrow.rs`.
@LeSingh1 LeSingh1 force-pushed the fix-isinstance-tuple-negative-narrowing branch from b5371c8 to 2a86c1b Compare May 15, 2026 22:10
@github-actions github-actions Bot added size/s and removed size/s labels May 15, 2026
@github-actions
Copy link
Copy Markdown

Diff from mypy_primer, showing the effect of this PR on open source code:

bokeh (https://github.com/bokeh/bokeh)
- ERROR src/bokeh/plotting/contour.py:308:31-36: Argument `Sequence[ColorLike] | str | tuple[int, int, int] | tuple[int, int, int, float] | tuple[str, ...]` is not assignable to parameter `palette` with type `tuple[str, ...]` in function `bokeh.palettes.interp_palette` [bad-argument-type]
+ ERROR src/bokeh/plotting/contour.py:308:31-36: Argument `Sequence[ColorLike] | tuple[int, int, int] | tuple[int, int, int, float] | tuple[str, ...]` is not assignable to parameter `palette` with type `tuple[str, ...]` in function `bokeh.palettes.interp_palette` [bad-argument-type]

streamlit (https://github.com/streamlit/streamlit)
- ERROR lib/streamlit/elements/media.py:488:28-40: Object of class `RawIOBase` has no attribute `tobytes`
- Object of class `str` has no attribute `tobytes` [missing-attribute]

pandera (https://github.com/pandera-dev/pandera)
- ERROR pandera/engines/geopandas_engine.py:151:29-58: Cannot index into `Series` [bad-index]

ibis (https://github.com/ibis-project/ibis)
- ERROR ibis/backends/polars/__init__.py:298:54-58: Argument `Iterable[Path | str] | str` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR ibis/backends/polars/__init__.py:298:54-58: Argument `Iterable[Path | str]` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
- ERROR ibis/backends/polars/__init__.py:301:54-58: Argument `Iterable[Path | str] | str | Unknown` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]
+ ERROR ibis/backends/polars/__init__.py:301:54-58: Argument `Iterable[Path | str] | Unknown` is not assignable to parameter `obj` with type `Sized` in function `len` [bad-argument-type]

strawberry (https://github.com/strawberry-graphql/strawberry)
- ERROR strawberry/extensions/mask_errors.py:61:34-55: Object of class `ExecutionResult` has no attribute `initial_result` [missing-attribute]

static-frame (https://github.com/static-frame/static-frame)
- ERROR static_frame/core/util.py:2491:63-66: Type `bool` is not iterable [not-iterable]
- ERROR static_frame/core/util.py:2491:63-66: Type `date` is not iterable [not-iterable]
- ERROR static_frame/core/util.py:2491:63-66: Type `int` is not iterable [not-iterable]
- ERROR static_frame/core/util.py:2491:63-66: Type `integer` is not iterable [not-iterable]

werkzeug (https://github.com/pallets/werkzeug)
- ERROR src/werkzeug/datastructures/structures.py:42:23-33: Yielded type `tuple[tuple[K, V] | K, list[V] | tuple[V, ...] | V | Unknown]` is not assignable to declared yield type `tuple[K, V]` [invalid-yield]
+ ERROR src/werkzeug/datastructures/structures.py:42:23-33: Yielded type `tuple[tuple[K, V] | K, V | Unknown]` is not assignable to declared yield type `tuple[K, V]` [invalid-yield]

kornia (https://github.com/kornia/kornia)
- ERROR kornia/models/depth_estimation/base.py:62:35-45: Object of class `list` has no attribute `cpu` [missing-attribute]
- ERROR kornia/models/depth_estimation/base.py:63:46-59: Object of class `list` has no attribute `device` [missing-attribute]
- ERROR kornia/models/depth_estimation/base.py:63:67-79: Object of class `list` has no attribute `dtype` [missing-attribute]
- ERROR kornia/models/segmentation/base.py:183:40-59: Object of class `list` has no attribute `size` [missing-attribute]

scrapy (https://github.com/scrapy/scrapy)
- ERROR scrapy/http/headers.py:55:22-27: `Iterable[_RawValue] | bytes | int | str` is not assignable to variable `_value` with type `Iterable[_RawValue]` [bad-assignment]
+ ERROR scrapy/http/headers.py:55:22-27: `Iterable[_RawValue] | int` is not assignable to variable `_value` with type `Iterable[_RawValue]` [bad-assignment]
- ERROR scrapy/http/headers.py:57:22-29: `list[Iterable[_RawValue] | bytes | int | str]` is not assignable to variable `_value` with type `Iterable[_RawValue]` [bad-assignment]
+ ERROR scrapy/http/headers.py:57:22-29: `list[Iterable[_RawValue] | int]` is not assignable to variable `_value` with type `Iterable[_RawValue]` [bad-assignment]

spark (https://github.com/apache/spark)
- ERROR python/pyspark/sql/pandas/conversion.py:1028:59-70: Object of class `list` has no attribute `json` [missing-attribute]
- ERROR python/pyspark/testing/pandasutils.py:464:55-70: Object of class `Index` has no attribute `to_pandas` [missing-attribute]

@github-actions
Copy link
Copy Markdown

Primer Diff Classification

✅ 5 improvement(s) | ➖ 4 neutral | 9 project(s) total | +6, -15 errors

5 improvement(s) across pandera, strawberry, static-frame, kornia, spark.

Project Verdict Changes Error Kinds Root Cause
bokeh ➖ Neutral +1, -1 bad-argument-type
pandera ✅ Improvement -1 bad-index pyrefly/lib/alt/narrow.rs
ibis ➖ Neutral +2, -2 bad-argument-type
strawberry ✅ Improvement -1 missing-attribute pyrefly/lib/alt/narrow.rs
static-frame ✅ Improvement -1 not-iterable narrow_is_not_instance()
werkzeug ➖ Neutral +1, -1 invalid-yield
kornia ✅ Improvement -4 missing-attribute pyrefly/lib/alt/narrow.rs
scrapy ➖ Neutral +2, -2 bad-assignment
spark ✅ Improvement -2 missing-attribute pyrefly/lib/alt/narrow.rs
Detailed analysis

✅ Improvement (5)

pandera (-1)

This is a clear improvement. The PR fixes a bug in negative isinstance narrowing for tuple targets. The isinstance(coerced_data, (GeometryArray, pd.DataFrame)) check on line 142 should narrow coerced_data in the else branch to exclude both GeometryArray and pd.DataFrame. Before the fix, the intersection-based approach could reintroduce one of these types, leading to a false bad-index error on line 151. The error message specifically states 'Cannot index into Series' - this refers to pd.Series, which is one of the types in the GeoPandasObject union. After the isinstance check on line 142 filters out GeometryArray and pd.DataFrame, the remaining types in the else branch should be pd.Series | gpd.GeoSeries | gpd.GeoDataFrame. The bad-index error on pd.Series suggests that the type checker's stubs for pd.Series.__getitem__ don't accept the boolean Series/array being used as an index. With the fix, the sequential subtraction correctly removes both GeometryArray and pd.DataFrame from the type, and the type checker no longer reports the false positive - likely because the remaining types' indexing signatures are properly recognized, or because the narrowing now works correctly to determine the actual types that reach line 151.
Attribution: The change to narrow_is_not_instance in pyrefly/lib/alt/narrow.rs fixed the sequential subtraction of isinstance tuple targets. Previously, when checking isinstance(coerced_data, (GeometryArray, pd.DataFrame)), the negative branch (else/after) would incorrectly reintroduce types that overlap with multiple tuple elements due to the intersection-based approach. The new sequential subtraction correctly removes both GeometryArray and pd.DataFrame from the type of coerced_data in the negative branch, which means at line 151 the type is properly narrowed to types that support boolean indexing, eliminating the false bad-index error.

strawberry (-1)

This is a clear improvement. The PR fixes a bug in negative narrowing for isinstance checks with tuple targets. In mask_errors.py, line 58 checks isinstance(result, (GraphQLExecutionResult, StrawberryExecutionResult)). The else branch (line 60-61) should have result narrowed to exclude those two types. Before the fix, the intersection-based approach in narrow_is_not_instance was reintroducing types that overlapped with multiple tuple elements, causing result in the else branch to still include ExecutionResult types that lack initial_result. The fix correctly subtracts each tuple element sequentially from each union member, so the else branch now properly excludes the isinstance-matched types, and the initial_result attribute access is valid on the remaining type.
Attribution: The change to narrow_is_not_instance logic in pyrefly/lib/alt/narrow.rs fixed the sequential subtraction of tuple elements in isinstance checks. Previously, the code computed per-target results and intersected them, which reintroduced union members that overlapped with multiple tuple elements. The fix walks each union member once and subtracts every tuple element sequentially. This directly fixed the narrowing in the else branch of isinstance(result, (GraphQLExecutionResult, StrawberryExecutionResult)) on line 58 of mask_errors.py, which eliminated the false missing-attribute error on line 61.

static-frame (-1)

This is a clear improvement. The PR fixes a bug in negative isinstance narrowing for tuple targets. The removed error was a false positive — bool was incorrectly remaining in the type union at line 2491 because the old intersection-based approach for isinstance(key, (str, bytes)) and isinstance(key, INT_TYPES) negative branches was reintroducing subtracted types. With the fix, sequential subtraction correctly narrows the type, and bool (a subclass of int) is properly removed by the isinstance(key, INT_TYPES) check's negative branch.
Attribution: The change to narrow_is_not_instance() in pyrefly/lib/alt/narrow.rs switched from computing per-target subtraction results and intersecting them, to sequential per-element subtraction from each union member. This fixed the reintroduction of overlapping types (like bool being a subclass of int which overlaps with tuple elements), correctly removing bool from the type union before line 2491.

kornia (-4)

These were clear false positives caused by a bug in pyrefly's negative isinstance narrowing for tuple targets. The parameter images: Union[torch.Tensor, list[torch.Tensor]] is checked with isinstance(images, (list, tuple)) on lines 48-53. In the else branch (lines 62-64), images must be torch.Tensor. The old code failed to subtract list properly when the isinstance target was a tuple, leaving list[torch.Tensor] in the narrowed type. The same pattern applies to kornia/models/segmentation/base.py:183 where .size() was incorrectly flagged on list. The PR fix correctly handles sequential subtraction of tuple elements, resolving all four false positives.
Attribution: The fix in pyrefly/lib/alt/narrow.rs in the narrow_is_not_instance logic changed from computing per-target subtraction results and intersecting them, to sequentially subtracting each tuple element from each union member. Previously, intersect_impl(list[torch.Tensor], list) would reintroduce list[torch.Tensor] through subset-based intersection when the isinstance target was a tuple (list, tuple). The sequential subtraction approach correctly removes both list and tuple from the union, leaving only torch.Tensor.

spark (-2)

Both errors were false positives caused by a bug in pyrefly's isinstance negative narrowing for tuple targets. When isinstance(schema, (list, tuple)) was in the negative branch, the old code failed to fully subtract list from Union[StructType, List[str]], leaving list in the type at line 1028. Similarly, isinstance(right, (pd.DataFrame, pd.Index, pd.Series)) in the negative branch failed to subtract all three pandas types, leaving pd.Index (which lacks to_pandas()) in the type at line 464. The PR's fix to sequential subtraction correctly resolves both cases.
Attribution: The change to narrow_is_not_instance in pyrefly/lib/alt/narrow.rs replaced the per-target-intersect approach with sequential per-element subtraction. This fixed the negative narrowing for isinstance(x, (A, B, ...)) tuple targets, which previously reintroduced subtracted types through subset-based intersection. This directly eliminated both false positive missing-attribute errors.

➖ Neutral (4)

bokeh (+1, -1)

Same errors at same locations with same error kinds — message wording changed, no behavioral impact.

ibis (+2, -2)

Same errors at same locations with same error kinds — message wording changed, no behavioral impact.

werkzeug (+1, -1)

Same errors at same locations with same error kinds — message wording changed, no behavioral impact.

scrapy (+2, -2)

Same errors at same locations with same error kinds — message wording changed, no behavioral impact.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (4 heuristic, 5 LLM)

@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented May 19, 2026

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D105741367.

Copy link
Copy Markdown
Contributor

@stroxler stroxler left a comment

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented May 22, 2026

@yangdanny97 merged this pull request in 92413ec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

if isinstance(val, (str, bytes)) ignored

2 participants