Leibniz-HBI / smormlpy

A cookiecutter skeleton for SMO/MRML python tools.
MIT License
0 stars 0 forks source link

fix: document decoupling and implement in example #18

Open FlxVctr opened 1 year ago

FlxVctr commented 1 year ago

https://github.com/Leibniz-HBI/smormlpy/blob/fdf5624f16abf19f6ba459ce0df89ca112f0c1ac/%7B%7B%20cookiecutter.project_slug%20%7D%7D/%7B%7B%20cookiecutter.project_slug%20%7D%7D/main.py#L6

Can we find a different solution for this? If pdoc3 is not compatible with click, maybe we should use something else?

pekasen commented 1 year ago

Actually IMHO we should enforce a decoupling between our tools' implementation and the UI (click in this case) even more:

May be we should further expand on these issues in the documentation? Which leads to another question: how exactly do we arrive at those best practices? Should we talk about that a bit more in the team? And in which form do we document our conclusions?

FlxVctr commented 1 year ago

We arrive at best practices from project to project, I'd say. IMO this cannot be predicted or conceptualized in advance and will change over time anyway.

From my understanding, smormlpy was meant for CLIs only. Our best practice is click there, so rather then decoupling I'd rather start a new template for other UIs. Or do you mean decoupling within a project? That would make sense. (Edit: I think that's what you meant, so yeah, go ahead, if so.)

FlxVctr commented 1 year ago

To answer the questions: Yes, we might want to discuss this from time to time in the MRML meeting, but the best place to discuss and document our conclusions re. best practices is here in the issues, I think.

FlxVctr commented 1 year ago

Regarding usability, if we want to further down this road, it has to be better documented for newbies. Also, the problem with the implemented function is right now, that parameters have to be updated in the internal click function if any are added to the implementation function. This might be solvable by a generated internal function, but that starts to feel like overkill.

pekasen commented 1 year ago

We arrive at best practices from project to project, I'd say. IMO this cannot be predicted or conceptualized in advance and will change over time anyway.

From my understanding, smormlpy was meant for CLIs only. Our best practice is click there, so rather then decoupling I'd rather start a new template for other UIs. Or do you mean decoupling within a project? That would make sense. (Edit: I think that's what you meant, so yeah, go ahead, if so.)

Yup, I was speaking of internal decoupling and speaking of the command line interface as a user interface... not a graphical interface. Decoupling in this context IMHO is paramount, as it actually makes testing and maintaining the implementations easier.

This might be solvable by a generated internal function, but that starts to feel like overkill.

Agreed, although the click-wrapped function and the actual implementation do not need to be 100% mirrors of one another. E.g. in dabapush all of the internal logic is encapsulated in a class and the CLI calls the appropriate methods of the invoked command. tegracli on the other hand does the internal logic in async functions which are run inside the click command (which coming to think about it, may be not within our best practice).

pekasen commented 1 year ago

After looking at it: docstrings and implementation is not in sync. Will clarify and fix it.