Closed richardjgowers closed 2 years ago
I suspect everything was broken here, so you probably haven't broken anything in bringing this in. I also don't have anyone on my end ready to review, so I'd be happy to have you take over this repo entirely if you want to own it (and you can appoint your own reviewer). Grab me via my calendly if you'd like to talk directly about it.
The other thing that might/might not be relevant is that the Cresset/Edinburgh/BioSimSpace folks are using a fork of this already (and we recently brought their stuff back in) so it's somewhat up to you as to whether you find a way to share infrastructure with them or take this in a different direction.
Hiya, yes it would be good to keep @JenkeScheen in the loop so we can all still keep working with the same LOMAP version
@jmichel80 do you think he'd want to review PRs?
Hi folks, @davidlmobley yes I'd be happy to. LOMAP has been part of my daily research for a several months now so would be nice to be involved.
Thanks for your work on this. LOMAP is currently the only vendored package within BioSImSpace so it would be fantastic if there was a working conda-installable version that we could use instead. Let me know if you need any input from our end.
Cheers.
Is a review for this PR still needed, considering https://github.com/OpenFreeEnergy/Lomap/issues/6? Seems I might be slightly out of the loop.
On another note, I'd be happy to do a quick check that generated networks are identical between lomap1 and lomap2 at some point; I have code in place that aggregates edges for (almost) all congeneric series in https://github.com/openforcefield/protein-ligand-benchmark/tree/master/data using lomap1, it shouldn't be too difficult to compare with lomap2.. Especially with the new conda-forge release! (we can continue the discussion on this elsewhere if needed).
@JenkeScheen yes sorry, we ended up keeping our fork of lomap (aka lomap2
on conda-forge) and we'll be maintaining it from there. You should be able to run your checks against the conda-forge package, that's live now.
Do you guys want to do a PR here to update the README.md to point to that as the maintained version, or should I do it?
This PR tries to make Lomap usable as a library (rather than solely as a command line tool).
Changes made:
pkg_resources
to make them more portableDBMolecules
no longer callsargparse
, instead just takes arguments like a more normal classlomap.dbmol._startup_inner
as a non-command line way of invoking lomap (lomap.dbmol.startup
(the CLI entrypoint) now just parses command line args and proceeds to this inner function)There's quite a few tests failing, but I think these were failing before I started tinkering, (maybe due to recent changes?) so some archaeology is required to try and restore the original regression tests / correct the desired behaviour.
Also it's not clear if the PR should go into this repo, or if OFE will be "adopting"/forking Lomap going forwards.