search

marrink-lab / vermouth-martinize

Describe and apply transformation on molecular structures and topologies
Apache License 2.0
89 stars 40 forks source link

Make martinize2 accessible in python (see Issue #360) #405

Closed OLaprevote closed 1 year ago

OLaprevote commented 2 years ago

Changes done :) Yes, I thought so for Sphinx. I'm curious too, as you mentioned in a worst case scenario it's just removing a few lines. Yup, the duplicated docstring is a pain. Looking into the problem right now: I have some ideas on how to proceed.

One thing: I left the init.py untouched. That means currently if anyone wants to use the python function they have to write from vermouth.martinize2 import martinize2 (or import vermouth.martinize2). Would you rather have it more direct?

pckroon commented 2 years ago

Thanks :) There's something going wrong with passing filenames, and there's also something fishy with the docs that I can't reproduce locally... I'll see what's going on there.

Yup, the duplicated docstring is a pain. Looking into the problem right now: I have some ideas on how to proceed.

I was thinking of the inspect module. That has the functionality to create/change signatures and docstrings on the fly. And the argparse object should have all the required info to fill the blanks. But like I said, it's out of scope for this PR. Could be a nice second one though ;)

One thing: I left the __init__.py untouched. That means currently if anyone wants to use the python function they have to write from vermouth.martinize2 import martinize2 (or import vermouth.martinize2). Would you rather have it more direct?

I think this is fine for now. We can always add it to the __init__.py at a later point.

pckroon commented 2 years ago

Ok, small issue. The docstring style you use requires sphinx > 3.1.2. If I update my sphinx to the latest (4.4.0) I get about a million warnings. I'll open a separate PR to fix those, and then I think I'll come back here...

pckroon commented 2 years ago

I'm going to try closing and reopening this PR to get the CI to run. Hopefully.

OLaprevote commented 2 years ago

I've found a few mistakes in my docstrings as well while getting the automated documentation to work, e.g. missing colons. That may be one of the reason it doesn't pass the test. The corrections on the docstring and the automated documentation are right now one and only commit on my local computer: as soon as I have a bit of time I split them and push only the docstring changes.

pckroon commented 2 years ago

Ok great :) I'll patiently wait for those commits then!