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

isinstance(inst, Instruction) returns False for the types that make up the Instruction enum #148

Open
MarquessV opened this issue Feb 24, 2023 · 0 comments

Comments

@MarquessV
Copy link
Contributor

MarquessV commented Feb 24, 2023

In Python, it would be helpful to easily check if some object is one of the types that make up an Instruction without it necessarily being the Instruction class itself.

Typically, types are checked with the isinstance() function like so:

from quil.instructions import Gate, Instruction

gate = Gate(...)
isinstance(gate, Instruction)

As it stands, the isinstance(gate, Instruction) call would return False, because Gate and Instruction are distinct types.

In Python, you might handle this with inheritance by defining Gate as class Gate(Instruction). However, this isn't possible with Rust and pyo3 because an enum cannot be a base class or derive itself from one (see last paragraph in this section of the pyo3 user guide).

If we wanted to handle this in pure Rust, I see two options:

  1. Define a minimal base class that every instruction type could extend. This would give all of the individual instruction types a common type to use isinstance with. However, because the Instruction enum can't inherit from other classes, it couldn't be made to pass the same isinstance check, adding some counter-intuitive weirdness to the API.

  2. Wrap the PyInstruction enum in a struct like:

PyInstructionBase(PyInstruction)

We can the use the wrapping class to extend each of the individual instruction classes and as the public facing Instruction type. However, all of the is/to/as/from methods would be buried in an inner field of that class, making the API nested in an odd way:

i = Instruction(Gate(...))
i.inner().is_gate()

This would also add a good amount of boiler plate to the __new__ methods for each of the instruction variants.

If we expand our horizons to Python, we can consider overriding the __instancecheck__ dunder method. This method lets you define the logic of isinstance on a metaclass. Metaclasses can't be defined in pyo3 (source), which is why this is a Python only solution.

The implementation would look something like this:

from quil.instructions import Instruction as BaseInstruction
class AbstractInstructionMeta(abc.ABCMeta):
    @classmethod
    def __instancecheck__(cls, __instance: Any) -> bool:
        if isinstance(__instance, BaseInstruction):
            # required because unit variants can't be initialized from themselves in __new__
            return True
        try:
            BaseInstruction(__instance)
            return True
        except Exception:
            return False


class Instruction(BaseInstruction, metaclass=AbstractInstructionMeta):
    pass

The Instruction class exported from this module would work as desired with isinstance and otherwise behave the same as the Instruction enum we've exported from the raw bindings. The complication in this approach comes from the build process. It's not clear to me that we can easily re-export Python code from, say, qcs-sdk-rust, like we can pure Rust code. We may need to introduce a build script that copies over everything from quil-py. This may need to be done for type hints anyways.

All of that said, the motivation for this is supporting pyQuil. We could choose to not support this, and rely on this workaround that commandeers AbstractInstruction.

@MarquessV MarquessV changed the title Is it possible to do isinstance(inst, Instruction) returns False for the types that make up the Instruction enum Feb 24, 2023
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

No branches or pull requests

1 participant