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

Missing destroyed attribute on OphydObject #1198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wakonig
Copy link

@wakonig wakonig commented Jul 2, 2024

Description

While OphydObject implements the destroy method, it does not provide the _destroyed attribute. The latter is only implemented on Device. Given the "_", I assume it is not meant to be used directly, yet I'm not aware of another way to check whether an object was destroyed. In the absence of a more elegant solution, I'd suggest to add the _destroyed attribute to the OphydObject class to make available e.g. to a SoftPositioner.

Related Issues

#1095

Type of Change

  • Added _destroyed attribute to OphydObject
  • Set _destroyed to True after a call to destroyed

@tacaswell
Copy link
Contributor

We are tracking if destroy has been called in Device and Signal to prevent resuscitating them out of an abundance of caution that we will be able to reliably get any epics callbacks and connections configured correctly again. For soft signals / positions it is not clear to me that we need to prevent their re-use the same way we do with things talking to an underlying control system.

Why do you need to check if an object has been destroyed?

@wakonig
Copy link
Author

wakonig commented Jul 2, 2024

It is more a question of consistency: This caught me very much by surprise that although it implements the destroy method, it lacks the ability to check if the destroy method was called. If this should only be available for classes that truly need it (e.g. Epics-based classes), I'd guess Device and Signal shouldn't have them either and instead they should move to e.g PVPositioner and EpicsSignalBase?

@tacaswell
Copy link
Contributor

I think nothing has public API to check if destroy has been called (other than implicitly via operations failing in some cases). It is an implementation detail that some classes keep track of this as internal state and keep them selves from being un-destroyed.

If we want to lift "have I been destroyed" up to public state and add it to the protocols, we can talk about that. However, I am still not clear why you would check if something has been destroyed or not (it appears the only place we call it in ophyd is in the test suite where we create and discard many objects, but when we call obj.destroy it is on the path to dropping all references so there is nothing left to check) and adding unused private state does not seem useful to me.

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