mxcube / mxcubecore

Backend used by MXCuBE
http://mxcube.github.io/mxcube/
GNU Lesser General Public License v3.0
11 stars 51 forks source link

Add custom docstring sections for hardware objects #947

Closed fabcor-maxiv closed 6 days ago

fabcor-maxiv commented 1 week ago

This adds two custom docstring sections:

I believe there is some "bikeshedding" to be made:

The Emits was actually introduced in https://github.com/mxcube/mxcubecore/pull/866 which is not merged yet (cc @meguiraun, see below).

The Hardware object properties was requested by @elmjag for a change that is not in GitHub (yet), and I believe it is a good idea.

@meguiraun for info, I also reused the AbstractActuator changes that you made in https://github.com/mxcube/mxcubecore/pull/866 as base for an example on how to use these in docstrings. But now, this might cause merge conflict...

You can preview how it renders at:

marcus-oscarsson commented 1 week ago

Nice, we should be careful with the Hardware object properties because the configuration will be defined by a Pydantic model and the properties accessed via .config or directly via the hardware object it self (if its a role). In this case the Hardware object properties would be on the configuration object I guess.

I don't mind if we add this in the mean time, but it does mean that we will, probably, need to change it later.

fabcor-maxiv commented 1 week ago

Nice, we should be careful with the Hardware object properties because the configuration will be defined by a Pydantic model and the properties accessed via .config or directly via the hardware object it self (if its a role). In this case the Hardware object properties would be on the configuration object I guess.

I don't mind if we add this in the mean time, but it does mean that we will, probably, need to change it later.

Right... I did not have the Pydantic story fully in mind. I am still not clear on how it will look like. But maybe with the Pydantic models, it will be self-documenting somehow... I do not know. I would be interested in seeing concrete examples.

I guess I do not mind changing it back to something else when it is time, and anyway I doubt anyone will have time/energy to write many such docstrings before the migration : D

rhfogh commented 1 week ago

I agree with @marcus-oscarsson

marcus-oscarsson commented 6 days ago

Its still a very preliminary draft but it would/could look something like this:

https://github.com/mxcube/mxcubecore/blob/8ba39dcbb663a459be89726971a2ba842d9c647e/mxcubecore/HardwareObjects/abstract/AbstractDetector.py#L66

The model could be better specified through Field and given a description as well (which is not yet the case in the example). So it is indeed to a certain extent self describing but one could of course imagine a class level documentation of the model.

https://github.com/mxcube/mxcubecore/blob/8ba39dcbb663a459be89726971a2ba842d9c647e/mxcubecore/model/configmodel.py#L29

fabcor-maxiv commented 6 days ago

Seems like this could help, looks promising: https://pypi.org/project/autodoc_pydantic/

I close this pull request for now.

marcus-oscarsson commented 6 days ago

Wow, thats seems nice. I think the emit documentation is still very nice though !

fabcor-maxiv commented 6 days ago

I think the emit documentation is still very nice though !

I will try to take over https://github.com/mxcube/mxcubecore/pull/866 then, that would make more sense.

marcus-oscarsson commented 6 days ago

Ok