Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Hide parts of the knot_extensions API? #15355

Open
sharkdp opened this issue Jan 8, 2025 · 4 comments
Open

[red-knot] Hide parts of the knot_extensions API? #15355

sharkdp opened this issue Jan 8, 2025 · 4 comments
Labels
red-knot Multi-file analysis & type inference

Comments

@sharkdp
Copy link
Contributor

sharkdp commented Jan 8, 2025

Quoting some comments that were spread over multiple threads in this PR:

@MichaReiser

What are your thoughts on whether we should gate this behind a feature or make it testing only?

@sharkdp

Seeing that mypy and pyre have similar public APIs, I could imagine (part of) this becoming a public API for red knot that isn't feature-gated? The Intersection/Not constructors and static_assert are potentially interesting for users which don't care too much about compatibility with other type checkers (cf python/typing#213). Some less-interesting parts like is_singleton could also be moved to knot_extensions.internal maybe?

Do we need to decide this right away? If not, can this stay public for now, or should we approach it very carefully and hide everything (by default)?

@MichaReiser

I'm generally leaning toward keeping the API intentionally limited and only expose features that we want to support long term (and consider part of the public API) because users will using them and it's very easy that we forget to revisit this decision before the release (unless we note it down somewhere?)

@carljm

For what it's worth, there's nothing in the API added in this PR that I have reservations about supporting long-term as public API, in principle. Of course it's always possible that we realize we made mistakes in details of how we defined or named the API, and we want to change these things in future and potentially have to take backward-compatibility into account with those changes. But this is inevitable.

And another comment by @carljm regarding how this could be implemented technically:

I think whatever feature gating we do should probably not happen at the Rust implementation level (that is, trying to turn off or on certain Rust code paths in type inference), but rather at the Python level; that is, in the type stub for the knot_extensions module. This could mean only conditionally including the type stub for knot_extensions at all, or having some conditional definitions of things in the type stub. If you can't import the known-function and known-instance types that make up the type API, then it doesn't matter whether the code paths exist for those; you won't be able to enter those code paths anyway.

@sharkdp sharkdp added the red-knot Multi-file analysis & type inference label Jan 8, 2025
@AlexWaygood
Copy link
Member

I wonder if one way of doing this would just be to issue a diagnostic whenever we see an import from knot_extensions in code that we're checking (which warns you that it's (a) experimental and (b) doesn't exist at runtime), but have that diagnostic switched off by default in the context of our test suite?

@sharkdp
Copy link
Contributor Author

sharkdp commented Jan 14, 2025

I like that! I guess the only downside I can think of right now is that we might want to deprecate that lint rule eventually if we ever want to "stabilize" knot_extensions.

@MichaReiser
Copy link
Member

Would this be a regular lint that you can disable? If so, what category would it belong to long term?

I think it should not be a regular rule. We don't want it to show up in configurations etc. While I designed for this possibility, not everything is there yet to actually support this distinction.

@carljm
Copy link
Contributor

carljm commented Jan 14, 2025

I'm not sure we need to do anything at all about this issue; I don't think it's a priority to spend time on it right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

No branches or pull requests

4 participants