Skip to content

Conversation

@AnOctopus
Copy link
Contributor

📚 Context

It came up that from __future__ import annotations allows us to use generic builtins, type union syntax, and lazy annotations, while still being compatible with 3.7. So let's start using them.

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe:

🧠 Description of Changes

  • This is a breaking API change
  • This is a visible (user-facing) change

🧪 Testing Done

  • Screenshots included
  • Added/Updated unit tests
  • Added/Updated e2e tests

@AnOctopus AnOctopus added the security-assessment-completed Security assessment has been completed for PR label Nov 19, 2022
@AnOctopus AnOctopus marked this pull request as ready for review November 19, 2022 00:32
Copy link
Contributor

@harahu harahu left a comment

Choose a reason for hiding this comment

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

Am realizing I was starting to repeat myself quite a bit here, so it's probably better to summarize.

Basically, there's a few more generic types you can replace, like tuples and sets.

That being said, great effort. I think this is a good idea, unless you have runtime uses for type annotations, which I don't think you do.

@AnOctopus
Copy link
Contributor Author

Good catches, wrt set and tuple. I'm also going with changing Optional to use the union op with None, though I'm open to reverting that if someone else strongly objects.

Copy link
Contributor

@willhuang1997 willhuang1997 left a comment

Choose a reason for hiding this comment

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

LGTM in general.

@AnOctopus AnOctopus merged commit 78d633a into streamlit:develop Nov 29, 2022
@AnOctopus AnOctopus deleted the chore/future-annotations-caching branch November 29, 2022 19:21
tconkling added a commit to tconkling/streamlit that referenced this pull request Nov 29, 2022
* develop:
  Use newer annotations features in caching modules (streamlit#5738)
  Add links for workflow failure slack messages (streamlit#5772)
  Refactor AppSession to simplify improving reconnect behavior (streamlit#5782)
  Fixing component-template typecheck failure (streamlit#5780)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants