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

inconsistent type issue of opx_driver.OPX.qua_program #187

Open
sherbrooke-equal1 opened this issue Feb 12, 2024 · 4 comments
Open

inconsistent type issue of opx_driver.OPX.qua_program #187

sherbrooke-equal1 opened this issue Feb 12, 2024 · 4 comments

Comments

@sherbrooke-equal1
Copy link

sherbrooke-equal1 commented Feb 12, 2024

the member qua_program returns an QUA program but it is used in get_prog() as if it were an actual QUA program instance

The recommended usage pattern is also to replace the method in the instance with a field containing the qua program instance and not with a callable method.

This causes type checking issues showing up in Pylance (VS Code) which complains about overwriting () -> QuaProgram with just QuaProgram, and the result of respecting the type hints information or as matter of fact leaving the method as-is would cause errors.

My recommendation would be to change this so that qua_program would always be a field created in the constructor and assigned to maybe _default_qua_program which is a @staticmethod.

Original code for context (with added type hints and comment at type error):

    # Empty method that can be replaced by your pulse sequence in the main script
    # This can also be modified so that you can put the sequences here directly...
    def qua_program(self) -> Program:
        """
        Custom QUA program

        :return: QUA program
        """
        with program() as prog:
            pass
        return prog

    # @abstractmethod
    def get_prog(self) -> Program:
        """Get the QUA program from the user"""
        prog = self.qua_program # <--- incorrect type
        return prog

@yomach
Copy link
Collaborator

yomach commented Feb 12, 2024

@sherbrooke-equal1 - I guess you're speaking on the opx_driver, right? I do not see any type hinting there in the main branch, are you on a side branch?

@TheoLaudatQM - Do you know?

In any case, get_prog(self) should return a QuaProgram

@sherbrooke-equal1
Copy link
Author

sherbrooke-equal1 commented Feb 13, 2024

with added type hints and comment at type error):

i have added the type hints myself to illustrate the problem (read Program as QuaProgram) although it might make the problem even clearer if they were added to the actual code

@yomach
Copy link
Collaborator

yomach commented Feb 13, 2024

Ok, I looked at it again, I was wrong and it does return a Program and not a QuaProgram.

I can't really see the issue... even when I add type hinting, but I'm using PyCharm.
Do you have a link to any VSCode issue about it? or a place that points to the recommended use pattern?

In any case, @TheoLaudatQM - is there any specific reason we went to this implementation of having an abstract method get_prog which points to another method called qua_program? I think that replacing qua_program by a field makes sense.

@yomach
Copy link
Collaborator

yomach commented Mar 19, 2024

@sherbrooke-equal1 - Can you please see if PR-190 fixes it?

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

2 participants