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

One option for adding generic types #56

Merged
merged 4 commits into from
Apr 2, 2024

Conversation

cwstryker
Copy link
Contributor

@cwstryker cwstryker commented Feb 6, 2024

It seems like there is a strong desire to implement better typing in the robotpy-commands-v2 library. There are potentially several ways to improve typing. The approach shown here uses TypeVar to create a generic type of ProfiledPIDController, named GenericProfiledPIDController. The generic type is defined in new typing.py file. The new generic type is then used in profiledpidsubsystem.py file as a type hint. See the file differences for more details.

Advantages:

  • Eliminates the use of type Unions.
  • Should be easy to maintain as the WPI C++ library evolves over time.

commands2/typing.py Outdated Show resolved Hide resolved
@cwstryker
Copy link
Contributor Author

If this sounds like a good path forward, I'll work on adding more generic types are time permits.

@virtuald
Copy link
Member

This seems great. Any reason not to merge it in at this point?

@cwstryker
Copy link
Contributor Author

It's ready to merge.

… Python naming convention. Added Generic annotations and a Protocol to profiledpidcommand.py.
…Aliases (FloatSupplier & FloatOrFloatSupplier). Updated UseOutputFucntion protocol based on testing.
@virtuald virtuald force-pushed the cwstryker/generic_types branch from 07af9dc to 604af69 Compare April 2, 2024 14:34
@virtuald virtuald merged commit 9ec9148 into robotpy:main Apr 2, 2024
19 checks passed
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.

2 participants