Unified program options in pyloos

[agrossfield]

Hi all,

I've been thinking about making the process of doing command line options in pyloos simpler, in a manner similar to what we did for the c++ tools (put a wrapper on top of the module we're using to process the options, so that you can import groups of commonly used options.

I made a preliminary implementation in a new branch, python-options. I don't want to make a pull request because it's nowhere near ready to be merged. Specifically, there's an options.py module in the pyloos directory (not Packages/PyLOOS), and an example of usage in Packages/PyLOOS/cylindrical-density.py

The biggest ugliness I see is in handling --fullhelp options. Right now, I still need to define the class in the application, because otherwise I can't see how to get the application-specific function embedded, and that's a pretty ugly thing.

Anyway, I'd appreciate comments and (helpful) suggestions. @tromo ? @lgsmith ?

-- Alan

[lgsmith]

One of the reasons that I like argparse is because it feels a lot like the options framework we have in c++ but simplified. I must admit I don't know what the click that @tromo mentioned is. I would want it to be as similar to PO as possible, because at least for me learning argparse helped me learn PO.

I would have either done this mostly like you're doing it (but note, as a module I don't think the line parser = argparse.ArgumentParser() Is needed, since the user would have to write loos.options.parser to access it anyway, and it's just an empty parser), or I would have done the python subclass thing and made a class containing a parser with all the methods you're currently defining as functions.

It seems to me like subclassing parser with something that has a bunch of loos specific methods is a little nicer because then the pyloos user doesn't have to remember to import argparse and instantiate an empty parser on their own. In that way perhaps it's a bit more similar to our subclassing of PO. As written above, that homology between the C++ and Python codes is pretty virtuous, and while the python should strive to be cleaner and more pythonic I think it should not be so at the expense of totally losing the homology. As a corollary, if two things are roughly equally simple in python but one is more homologous to the loos C++ library we should prefer that.

As to how to do fullhelp, well, all fullhelp is in python tools is a triple quote string, so you'd just make a parser method that stores a string for printing upon calling fullhelp. The class's init method can start with that string as an empty string, or perhaps (to warn people that they need to feed it a string) something like 'there's no fullhelp yet'. You want the triple-quote string to be in the tool, so that autodocs tools that look for those can grab it.

[lgsmith]

Wait you are saying you can't have your custom subclass have a fullhelpstring as part of its state, and give your parser an option that is just a boolswitch that prints an argument string (self.fullhelp) then calls exit(0)?

[lgsmith]

Something like this:

import argparse as ap

class po:
    def __init__(self, parser: ap.ArgumentParser):
        self.parser = parser
        self.fullhelp = 'fullhelp'
    def add_fullhelp(self, fullhelp):
        self.fullhelp = fullhelp
        self.parser.add_argument('--fullhelp', action='store_true', default=False)
    def parse_args(self):
        args = self.parser.parse_args()
        # postprocess args here; error checks, checks for needed behavior
        if args.fullhelp:
            print(self.fullhelp)
            exit(0)
        return args

[agrossfield]

That's different from how it currently works. I think what you're describing would work, although it's not the way argparse is intended to be used (the way we're doing it is, where you create a custom action).

[agrossfield]

actually, there's no reason for that to be a subclass. In your example, po just contains an ap.ArgumentParser object.

[lgsmith]

Yes, I decided that was simpler and wrote it as a wrapper instead.

[lgsmith]

Yes. I decided that was simpler.

…

On Wed, Apr 7, 2021 at 10:48 AM Grossfield Lab ***@***.***> wrote: actually, there's no reason for that to be a subclass. In your example, po just contains an ap.ArgumentParser object. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAZVYMVVBPLSFZAGKE6ZYTTHSEHPANCNFSM42QZYW5A> .

[agrossfield]

I pushed a second version today, as a RFC. Thoughts?

[lgsmith]

Noticed a small booboo in the argv length check (I believe). I just fixed it and pushed it back. This is basically what I was thinking about, although the length check that prints the parser autohelp statement seems like it belongs in the parse_args statement rather than init.

Something that may also be nice, and that I kind of wish the PO had (perhaps I should put it in) is sort of a verbose mode where it pukes up all the options it thinks it got to std error in the case of any kind of runtime issue or segfault. The pythonic equivalent of that (and of writing the 'header string, that almost all loos tools write) probably belongs in the argparse wrapper even though in the CPP they're separate.

[agrossfield]

You’re right, and I’ll add a generateHeader option.

…

________________________________ Dr. Alan Grossfield Dept of Biochemistry and Biophysics University of Rochester Medical Center Phone: 585 276 4193 http://membrane.urmc.rochester.edu https://orcid.org/0000-0002-5877-2789 Pronouns: He/his From: lgsmith ***@***.***> Reply-To: GrossfieldLab/loos ***@***.***> Date: Thursday, April 8, 2021 at 2:55 PM To: GrossfieldLab/loos ***@***.***> Cc: "Grossfield, Alan" <[email protected]>, Author ***@***.***> Subject: [EXT] Re: [GrossfieldLab/loos] Unified program options in pyloos (#43) Noticed a small booboo in the argv length check (I believe). I just fixed it and pushed it back. This is basically what I was thinking about, although the length check that prints the parser autohelp statement seems like it belongs in the parse_args statement rather than init. Something that may also be nice, and that I kind of wish the PO had (perhaps I should put it in) is sort of a verbose mode where it pukes up all the options it thinks it got to std error in the case of any kind of runtime issue or segfault. The pythonic equivalent of that (and of writing the 'header string, that almost all loos tools write) probably belongs in the argparse wrapper even though in the CPP they're separate. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_GrossfieldLab_loos_discussions_43-23discussioncomment-2D586323&d=DwMCaQ&c=4sF48jRmVAe_CH-k9mXYXEGfSnM3bY53YSKuLUQRxhA&r=49qnaP-kgQR_zujl5kbj_PmvQeXyz1NAoiLoIzsc27zuRX32UDM2oX8NQCaAsZzH&m=yp2Y8o9tXWt7fYdy-D_5KKipDLG0tWvt4SKWQknPqFw&s=ArU9DrdS5sFnDMxPLIJUBreWE3TxLHJalTqjkf8gkic&e=>, or unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ADH754QWFU7IU6PPBDSRH6DTHX3ZDANCNFSM42QZYW5A&d=DwMCaQ&c=4sF48jRmVAe_CH-k9mXYXEGfSnM3bY53YSKuLUQRxhA&r=49qnaP-kgQR_zujl5kbj_PmvQeXyz1NAoiLoIzsc27zuRX32UDM2oX8NQCaAsZzH&m=yp2Y8o9tXWt7fYdy-D_5KKipDLG0tWvt4SKWQknPqFw&s=xJDJ_9t6aQkAyKoi_j3dn0inx7UIzxf4m1vpnVPMsDA&e=>.

[lgsmith]

I'm waffling about the subclass vs wrapper thing, again. It would be kind of nice not to have to write loosparser.parser.add_argument(argjunk). It would be some added familiarity for people to not have to do that, I think.

I will try to write a version of the exact same options code as a subclass this evening, if I get a free minute. Then you can look at them both and decide.

[agrossfield]

RFC: take a look at the latest iteration of options.py
To get an idea of what it feels like in use, I wrote a new tool called contact_distance.py (basically, rmsds but comparing the matrix of residue-residue contacts instead of the RMSD). I'm moderately happy with it, but I want some feedback before I merge.

[agrossfield]

I'm leaning toward merging this in but not going back and retrofitting the other tools right away. Any objections?

[agrossfield]

I've made a pull request ( #49 ). I'll merge it next week if there are no objections