[Documentation Code Camp] docstrings for emitted signals

[meguiraun]

Initial work for documenting emitted signals. I initially focused on the abstract classes (not all done yet 🤪)

Example output:

[fabcor-maxiv]

Wouldn't it make more sense to document the emitted signals at the class level rather than at the method level?

If I use an actuator I want to know what signals it emits and when (under what circumstances, i.e. what is the meaning of the signal), but I do not need to know which of the actuator's methods actually emits the signal, do I?

And same thing, if I want to implement a new (concrete) actuator, I want to know what signals my actuator should emit, but I will choose which methods will actually emit those signals.

cc @elmjag

[beteva]

@meguiraun thanks for the work. Good start. There is one slight problem - I think AbstractAperture is an obsolete class, There will be very soon a new PR to handle aperture test and mock up, but they inherit from AbstractNState. In this context, would you mind if we see together which are the obsolete abstract classes.
@fabcor-maxiv I guess it is all right if we document the signals on the class level only. Actually this is the case right now, but certainly needs a bit of explanation which signal means what.

Maybe we should make a general signals documentation file, which lists all the "standard" signals, like valueChanged, limitsChanges, stateChanged, specificStateChanged, as well as the methods defined in BaseHardwareObjects.py - the emit method (namely its signature in order to explain how to emit a signal) and the force_emit_signals method, available for overloading . If we go for this, than we could explain the usage of these signals and in the classes only mention which are the emitted signals, and only document if there are other signals, specific for the class. What do you think?

[rhfogh]

I agree with @beteva - The biggest need is what are the general kinds of signals that are used to tie the application together, where they come from, and what they do. In a way the biggest question is not so much 'which signals does this class emit' but 'which signals should I listen for' and 'which signals do I need to emit in my own class?'.

[meguiraun]

okey, I see your point. I will leave the methods out.

yes, totally agree with a general documentation page (why, where in code are coming, some basic example, etc.), I had it in mind but I thought to start with documenting the code

[meguiraun]

1. Added a new md with signal documentation, very early, please @beteva @rhfogh have a look, and if you feel so commit any improvement
2. Updated, let me know if the "new" AbstracActuator class documentation is ok:

[fabcor-maxiv]

I do not think there is a BaseHardwareObject class. So I replaced with HardwareObject. But in most cases what we talk about is actually in the HardwareObjectMixin.

I tried the Sphinx/docutils cross-reference roles, we'll see if we get better links.

In principle we should be able to configure Sphinx to keep a copy of the source code, so that we can have links to source code in the documentation (not links to GitHub).

And in general, not only for (Sphinx) documentation, we should avoid links to a branch, but use permalinks that link to a specific commit.

[fabcor-maxiv]

In many of the modified docstrings, it feels like adding the correct type hints would be nearly as efficient (especially for the return values).

[rhfogh]

@fabcor-maxiv HardwareObjectMixin woudl be the right choice throughout. HardwareObjects inherit from either HardwareObject or HardwareObjectYaml, but both inherit from HardwareObjectMixin,.

[meguiraun]

preview in https://mxcubecore--866.org.readthedocs.build/en/866/dev/hwobj_signals.html

[beteva]

Very good job @meguiraun 👍
The device and the equipment signals are definitely deprecated. Quite some time ago there was a decision not to use Device and Equipment class any more.
You are right, we should remove the update signal and only use valueChanged.
You can also remove the xrf related signals - in my WIP for xrf I've changed them all to stateChanged

[marcus-oscarsson]

Well done ! very nice !. As Antonia already pointed out, we should really start to remove the Device and Equipment classes.

[meguiraun]

while the signals are still there, I simply marked as deprecated.
Also added queue and diffractometer

[beteva]

Very nice.
Apparently we need changes in many python files for better documentation. I suggest we see together with the other developers who does what, but I like the class description you've made for AbstractActuator. We should follow the same way.

[meguiraun]

AbstractBeam with docstrings in class here
AbstracActuator with docstrings outside class here

I can remove all changes to the abstract classes if it makes the other PR easier to handle

ping @beteva

[fabcor-maxiv]

AbstractBeam with docstrings in class here
AbstracActuator with docstrings outside class here

For me, this belongs in the class docstring : )

[meguiraun]

AbstractBeam with docstrings in class here
AbstracActuator with docstrings outside class here

For me, this belongs in the class docstring : )

I think same way, it seems its place

[beteva]

@meguiraun, agree the best place is the class docstrings.

[marcus-oscarsson]

Hey @meguiraun, do you have an update on this one, it seemed almost done ?

[meguiraun]

completely lost track of this one... so sorry...

If people are happy I am also happy. Not all the hwobjs are covered tough

[fabcor-maxiv]

Not all checks are successful. After this is fixed, I think it is fine to merge. And then I might do follow-ups as I had already started in #947.

[meguiraun]

all checks are green now!

[marcus-oscarsson]

Sorry my mistake. I must have missed the last comments, was this ready to be merged ?