Skip to content

Simple typed mux#262

Draft
daniel-montanari wants to merge 15 commits into
PyFixate:mainfrom
daniel-montanari:simple-typed-mux
Draft

Simple typed mux#262
daniel-montanari wants to merge 15 commits into
PyFixate:mainfrom
daniel-montanari:simple-typed-mux

Conversation

@daniel-montanari

Copy link
Copy Markdown
Collaborator

I like this approach more than #188
Annotated never felt quite right. I'm going to go ahead with the approach of making the user still have to define the mapping between signals and pins themselves. Having to chain Union and Literal together also was clunky as Literal is already kind of like a union of options.

In the future this will be easier to add features to, e.g pin typing, and hinting for constructing the map or tree.
The type hints can also eventually be digested to not have to repeat the pin list.

@clint-lawrence clint-lawrence left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks pretty good. The implementation is so much simpler, not having to rely on the run time type inspection to build the mux.

If it read it right, when there is a mismatch between the signals in the type and the signals used for the map_list, you would get a type error

type Signals = Literal["signal_1", "signal_2"]

class MyTypedMux(VirtualMux[Signals]):
    pin_list = ("x0", "x1")
    map_list = (
        ("signal_1", "x0"),
        ("bad_signal", "x1"),  # <- this would be a type error
    )

Have I understood that correctly?

Comment thread examples/jig_driver.py Outdated
Comment on lines +68 to +88
# the type keyword can be used to create reusable definitions
# otherwise Literal can be used directly
type Signals = Literal["signal_1", "signal_2"]

# note: the type keyword can't be used inside functions!
# generally we want to use type to avoid confusion around the type system
# this makes it clear we are creating something for typehinting
# e.g type MyInt = int - won't work in functions
# variable = int - is not obvious what the intent is and can behave differently depending on its scope


def do_some_stuff():
# otherwise the mux is created as normal
class MyTypedMux(VirtualMux[Signals]):
pin_list = ("x0", "x1")
map_list = (
("signal_1", "x0"),
("signal_2", "x1"),
)

mymux = MyTypedMux()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# the type keyword can be used to create reusable definitions
# otherwise Literal can be used directly
type Signals = Literal["signal_1", "signal_2"]
# note: the type keyword can't be used inside functions!
# generally we want to use type to avoid confusion around the type system
# this makes it clear we are creating something for typehinting
# e.g type MyInt = int - won't work in functions
# variable = int - is not obvious what the intent is and can behave differently depending on its scope
def do_some_stuff():
# otherwise the mux is created as normal
class MyTypedMux(VirtualMux[Signals]):
pin_list = ("x0", "x1")
map_list = (
("signal_1", "x0"),
("signal_2", "x1"),
)
mymux = MyTypedMux()
# note: the type keyword can't be used inside functions!
# generally we want to use type to avoid confusion around the type system
# this makes it clear we are creating something for typehinting
# e.g type MyInt = int - won't work in functions
# variable = int - is not obvious what the intent is and can behave differently depending on its scope
# the type keyword can be used to create reusable definitions
# otherwise Literal can be used directly
type MyTypedMuxSignals = Literal["signal_1", "signal_2"]
def do_some_stuff():
# otherwise the mux is created as normal
class MyTypedMux(VirtualMux[MyTypedMuxSignals]):
pin_list = ("x0", "x1")
map_list = (
("signal_1", "x0"),
("signal_2", "x1"),
)
mymux = MyTypedMux()

If you move the type definition closer and give it a less general name, I think that would help people see the connection between the two.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment thread examples/jig_driver.py
myrelay.multiplex("")
myrelay.multiplex("signal_1")
myrelay.multiplex("signal_2")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could also show a concrete example of the alternate you hint at in the comment above

class MyTypedMux(VirtualMux[Literal["signal_1", "signal_2"]]):
        pin_list = ("x0", "x1")
        map_list = (
            ("signal_1", "x0"),
            ("signal_2", "x1"),
        )

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

@daniel-montanari

Copy link
Copy Markdown
Collaborator Author

This looks pretty good. The implementation is so much simpler, not having to rely on the run time type inspection to build the mux.

If it read it right, when there is a mismatch between the signals in the type and the signals used for the map_list, you would get a type error

type Signals = Literal["signal_1", "signal_2"]

class MyTypedMux(VirtualMux[Signals]):
    pin_list = ("x0", "x1")
    map_list = (
        ("signal_1", "x0"),
        ("bad_signal", "x1"),  # <- this would be a type error
    )

Have I understood that correctly?

At the moment you won't get any help in the mux definition. You can still define the wrong signals. For the sake of getting something released without getting too bogged down in things I just wanted a simple interface that anyone can chuck into a mux to get autocomplete. If the definition is incorrect that's still on you.

Having the map_list as a tuple of strings where the position determines the meaning makes this more difficult.
There are probably some better data structures we could switch to for a new interface. Anything mutable also makes things harder. There are also issues around a str being a valid Sequence[str]

The error messages you get are also not very helpful with the current tuple structure
image

Once we've got a certain way forward I'm happy to add all the additional checks and type hinting.

@daniel-montanari

Copy link
Copy Markdown
Collaborator Author

Once we've got a certain way forward I'm happy to add all the additional checks and type hinting.

frozendict in python 3.15 looks promising, having to remind the typechecker that my dict of string literals is not just an arbitrary collection of strings feels clunky.

TypeForm might also help with what I'm currently getting stuck on with wanting to do TypeGuards on generic types (such as a user supplied signal definition instead of the base string). Currently the way I'm trying to get around this is by putting it inside the VirtualMux class. For now things are in a slightly awkward spot where I want to use the type system, but that means I can't use isinstance for any logical type checks.

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