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

type narrowing for agents and policies using generics #1443

Open
wakamex opened this issue Apr 26, 2024 · 2 comments
Open

type narrowing for agents and policies using generics #1443

wakamex opened this issue Apr 26, 2024 · 2 comments
Labels
enhancement A feature or refactor request

Comments

@wakamex
Copy link
Contributor

wakamex commented Apr 26, 2024

implement type narrowing all the way down to the lowest agent and policy level, through the use of generics
right now policies are typed as only a very high-level NoPolicy | BasePolicy which isn't useful at all

@wakamex wakamex added the enhancement A feature or refactor request label Apr 26, 2024
@dpaiton
Copy link
Member

dpaiton commented Apr 26, 2024

here's an example of where the type narrowing is not correct
image
This type is incorrect, which means I can't auto-complete any of the random_hold_policy functions or discover the arguments.

and another for config:
image

these should be random hold policy & random hold policy config, which would have correct autocomplete.

I have a TODO in base.py:

# TODO: We add this as a helper property so that subclasses can access the config without
# overwriting the __init__ function. The downside is that users of this member variable
# can't be type checked. There's probably a way to do this with generics instead of Any.

This hints that the solution is to use Generics, which I still think is correct. We need to revamp IHyperdrive to also use generics for this all to work.

The meta issue is that the python type system is fine with you narrowing the type to a base class, which removes all the metadata and content. So while it's not returning any type errors, it is not the behavior we want.

@dpaiton
Copy link
Member

dpaiton commented Jun 5, 2024

Move to medium priority bc the policy attribute is intended to be private & not accessed by the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or refactor request
Projects
None yet
Development

No branches or pull requests

2 participants