Skip to content

[ty] Support property tests for BoundSuper#18066

Closed
cake-monotone wants to merge 2 commits intoastral-sh:mainfrom
cake-monotone:cake/property-test-for-bound-super
Closed

[ty] Support property tests for BoundSuper#18066
cake-monotone wants to merge 2 commits intoastral-sh:mainfrom
cake-monotone:cake/property-test-for-bound-super

Conversation

@cake-monotone
Copy link
Contributor

Summary

This PR includes the following changes:

  • Fix incorrect disjoint_from behavior for BoundSuper types with dynamic parameters

  • Add Ty::BoundSuper cases to property tests

Test Plan

  1. Property tests for BoundSuper
  • QUICKCHECK_TESTS=10000 cargo test -p ty_python_semantic -- --ignored types::property_tests::stable
  1. Regression tests for incorrect disjoint_from behavior (mdtest)
  • cargo test -p ty_python_semantic --test mdtest -- disjoint

@github-actions
Copy link
Contributor

mypy_primer results

Changes were detected when running on open source projects
hydra-zen (https://github.com/mit-ll-responsible-ai/hydra-zen)
+ error[type-assertion-failure] tests/annotations/declarations.py:951:5: Actual type `FullBuilds[Unknown] | StdBuilds[Unknown]` is not the same as asserted type `FullBuilds[@Todo(Support for `typing.TypeAlias`)] | StdBuilds[@Todo(Support for `typing.TypeAlias`)]`
- Found 649 diagnostics
+ Found 650 diagnostics

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label May 13, 2025
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@carljm
Copy link
Contributor

carljm commented May 13, 2025

Can you run with QUICKCHECK_TESTS=1000000? We've seen infrequent failures that show up in CI later be consistently missed with lower quickcheck counts.

@cake-monotone
Copy link
Contributor Author

cake-monotone commented May 14, 2025

Can you run with QUICKCHECK_TESTS=1000000? We've seen infrequent failures that show up in CI later be consistently missed with lower quickcheck counts.

Okay!

@cake-monotone
Copy link
Contributor Author

cake-monotone commented May 14, 2025

I tried it out and got an error related to tuple.

It seems like tuple is never disjoint from any instance type, for example is_disjoint_from(tuple[()], int) returns False:
https://play.ty.dev/29a090cb-4019-4b2b-a554-8b80e86ff457

I think this is a related issue with #18004. Similar to #18004, we could temporarily work around this by disabling the following code:

2 => Ty::Tuple(
(0..*g.choose(&[0, 1, 2]).unwrap())
.map(|_| arbitrary_type(g, size - 1))
.collect(),
),

But the quality of the daily property tests might suffer a bit until it's properly fixed.
Shall we try disabling tuple generation for now?

@carljm
Copy link
Contributor

carljm commented May 16, 2025

It seems like tuple is never disjoint from any instance type

I think this is correct based on our current definition of the tuple type as including potential user subclasses of tuple, which might multiply-inherit from the other type as well. (Well, in the specific case of int we could consider it disjoint due to "multiple bases have instance layout conflict", but we don't model that yet.)

What specific property test does this end up failing?

@cake-monotone
Copy link
Contributor Author

cake-monotone commented May 17, 2025

What specific property test does this end up failing?

all_fully_static_type_pairs_are_subtype_of_their_union
[AlwaysFalsy, tuple[()] | ~super(int, int)]

which means tuple[()] | ~super(int, int) is expected to be a subtype of AlwaysFalsy | tuple[()] | ~super(int, int).

At this moment, AlwaysFalsy | tuple[()] | ~super(int, int) will become ~super(int, int)

its because:

  • AlwaysFalsy | tuple[()] = AlwaysFalsy (tuple[()] is a subtype of AlwaysFalsy)
  • AlwaysFalsy | ~super<int, int> = ~super<int, int> (super<int, int>.bool() == AlwaysTrue)

However, tuple[()] | ~super(int, int) is not a subtype of ~super(int, int)

its because:

  • tuple[()] is not subtype of ~super(int, int) (I guess it's wrong)

If my understanding is correct, two things need to be addressed:

  • The empty tuple literal tuple[()] should be handled more accurately.
  • The BoundSuper type should represent a set of special instances (not including custom child class of super). Therefore, is_disjoint_from(tuple[...], BoundSuper) should return true, unlike with typical instances.

@carljm
Copy link
Contributor

carljm commented Jun 10, 2025

Sorry for delay getting back to this. Our latest thinking is that we do want empty tuple to continue to be a subtype of AlwaysFalsy (and we will forbid user subclasses of tuple from overriding __bool__, to make this sound). I think this means that our handling of empty tuple is OK here, but we do need to fix that BoundSuper should be a more limited type, and disjoint from arbitrary instances. It seems like that should be sufficient here?

@AlexWaygood AlexWaygood removed their request for review June 10, 2025 15:55
@sharkdp
Copy link
Contributor

sharkdp commented Feb 3, 2026

I'm closing this due to inactivity. Please feel free to comment in case it should be re-opened.

@sharkdp sharkdp closed this Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments