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

References to properties error handling #82

Open
JacobHast opened this issue Nov 21, 2024 · 4 comments
Open

References to properties error handling #82

JacobHast opened this issue Nov 21, 2024 · 4 comments

Comments

@JacobHast
Copy link
Collaborator

If a quam component attribute is a reference to a property and this property throws an error, the error is ignored and the property instead returns the string reference. Is this intended behaviour? I my experience it makes it hard to debug custom components - if the attribute throws an error I think I would always like to see this error and have the program halt, rather than continuing with the string.

from quam import quam_dataclass, QuamRoot, QuamComponent

@quam_dataclass
class Root(QuamRoot):
    pass

@quam_dataclass
class Component(QuamComponent):
    x: int = "#./prop"

    @property
    def prop(self):
        assert 1 == 2
        return 42
    
root = Root()
root.component = Component()

print(root.component.x) # prints '#./prop'
print(root.component.prop) # raises AssertionError
@nulinspiratie
Copy link
Contributor

Yes this is actually intended behaviour, and I agree it's not ideal. Originally it raised an error, but this resulted in several other issues:

  • If somewhere in your QUAM you have a reference that cannot be resolved, it becomes difficult to understand where the issue lies
  • When loading QUAM, some components are loaded before others. If an earlier-loaded component references a later-loaded component, it can lead to errors if the referenced parameter is being accessed early on. This happened fairly frequently.
  • Other commands will stop working such as QUAM.print_summary(), making it harder to debug

As a result, I've opted to turn it into a warning instead. Do you see the warning? If not, it may be that you logging package needs to stop filtering out the warnings.

Turning it back into an error would be a breaking change, and could lead to the code of others suddenly not working anymore. Maybe a good in-between solution is to have a global flag that specifies whether it returns an error or warning? I would like it to be warning by default to avoid breaking changes

@JacobHast
Copy link
Collaborator Author

I see. I don't get warnings, not even when running the above example in a fresh kernel, so I'm not sure if I'm filtering out warnings, at least not intended. It would be nice to have the option to turn on errors - of course since I currently don't see the warnings I don't know if I'm in the group of people for which everything would stop working, but it would be good to find out.
Alternatively, would this be fixable by adding some kind of quam decorator to properties such that, when everything is "booting up", errors are not thrown, but once the machine is construct errors can be thrown?

@nulinspiratie
Copy link
Contributor

Both could work, bit since the decorator would still be a breaking change, I propose we handle this by adding a flag to specify whether it'll raise an error.

We can also look separately at why you aren't receiving a warning. I can also raise the log status from warning to error.

I'm planning to allow quam to have an optional config file. We can then easily add this as a flag

@nulinspiratie
Copy link
Contributor

Quick update: we're close to creating qualibrate-config, which will allow quam to also have config entries. Once that's ready we can add this as an optional config flag for quam

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