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

SubsetEnum behaviour is unexpected and/or not entirely documented #747

Open
shihab-dls opened this issue Jan 23, 2025 · 3 comments
Open
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@shihab-dls
Copy link

shihab-dls commented Jan 23, 2025

It seems that setting a signal with type SubsetEnum allows strings that are not attributes of the enum. Passing other types, like an int, will raise a ValueError
ex: ValueError: 1 is not a valid TestEnums

Is this expected behaviour? And if so, should this be better documented in the SubsetEnum definition?

Steps To Reproduce

Steps to reproduce the behavior:

class TestEnums(SubsetEnum):
    TEST = "str"


class TestDevice(StandardReadable):
    def __init__(self, prefix: str, name: str = ""):
        with self.add_children_as_readables():
            self.signal = epics_signal_rw(TestEnums, f"{prefix}TEST")
        super().__init__(name=name)


async def test_string(RE):
    with DeviceCollector(mock=True):
        my_filter = TestDevice("test-prefix")
    await my_filter.signal.set("test_str")
@shihab-dls shihab-dls added the bug Something isn't working label Jan 23, 2025
@coretl
Copy link
Collaborator

coretl commented Jan 24, 2025

This is expected behaviour. EPICS enums can be address by string value or integer index, but the integer index is generally more liable to change than the string value. For this reason we make SubsetEnum a str, Enum, and allow any other str to be put. The only exception to this is 2 member enums that are effectively True/False. For these you can use bool rather than SubsetEnum.

I'm a little confused about the code above, it looks like it should work, does it fail?

@shihab-dls
Copy link
Author

Quick Note: Sorry, I've just edited the issue, as I mistakenly put signal.set("str") instead of signal.set("test_str") to show any str is accepted; although, I think you addressed this in your comment anyways @coretl

@olliesilvester
Copy link
Contributor

As discussed, setting a SubsetEnum signal to an arbitrary string will fail at runtime on real devices, but not in unit tests due to how mock mode works. We should include in a docstring that this behaviour is different in unit tests

@shihab-dls shihab-dls self-assigned this Jan 24, 2025
@shihab-dls shihab-dls added documentation Improvements or additions to documentation and removed bug Something isn't working labels Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants