Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic generation of k_funcs from application XMLs #1

Closed
jessemapel opened this issue Feb 22, 2019 · 4 comments
Closed

Dynamic generation of k_funcs from application XMLs #1

jessemapel opened this issue Feb 22, 2019 · 4 comments

Comments

@jessemapel
Copy link
Contributor

jessemapel commented Feb 22, 2019

There are only a handful of apps supported via the k_funcs interface. It should be possible to generate the k_funcs programmatically similar to how the regular functions are generated.

All of the parameters are defined under <groups>, <group>, <parameter>. Any parameter with a <default>, <item> or <internalDefault> can be set as a kwarg. All of the other arguments can be positional arguments.

@rbeyer
Copy link
Owner

rbeyer commented Feb 22, 2019

Maybe I'm misunderstanding what you're saying, but I think you're saying two things:

  1. You could parse the $ISISROOT/bin/xml/*xml files to extract the 'default' parameters, and then automatically assign them to named kwarg parameters in the subprocess call out to the ISIS program.

  2. You could just take any positional argument, and assign them, in order, to the ISIS parameters, in the order they appear in the XML file.

I don't know that (1) is even needed (but I'm probably not understanding). Under the current paradigm, if you do

import kalasiris as isis

hi2isis(from_='some.img', to='some.cub')

You aren't specifying the other two arguments (LSBGAP or UNLUT), and when the factory-generated hi2isis function calls subprocess they aren't in the argument list passed to ISIS's getkey, and so they just default anyway.

If you only say hi2isis(from_='some.img') then the hi2isis function calls subprocess that way, and hi2isis from=some.img is passed to the ISIS hi2isis function, which requires a to= parameter, which fails, and triggers a subprocess.CalledProcessError exception, as expected.

For (2), I think you're saying that a call like either of these:

getkey_k(f, o, g, k1, k2)
getkey_k(f, o, g, k2, keyword=k1)

Should be equivalent to:

getkey(from_=f, objname=o, grpname=g, keyword=k1, keyindex=k2)

Basically assigning any keyword arguments to the right keyword, and then taking any positional arguments and assigning them, in order, to the keywords specified in the XML file that aren't already 'taken.'

That's sounds like a good idea to me, and could probably just be incorporated into the main kalasiris argument parsing logic without much fuss (although you'd have to engage some minimal-match to parameter names logic in Python rather than just letting ISIS handle it).

My intent with the _k functions was to provide an interface that is more natural, or provides more 'features' than the default Factory implementation.

For getkey_k when applied to ISIS cubes, they typically only have one Object, IsisCube, so the argument pattern's first three positional arguments map to FROM, GRPNAME, and KEYWORD because 90% of the time, that's how you use it on cubefiles. If you just did the automatic positional argument outlined above (which I think would still be globally useful), you'd always have to specify that OBJNAME parameter, since it comes second. Also, rather than returning the subprocess.CompletedProcess like the Factory-generated functions, it returns the stripped stdout, again, which is what people mostly run getkey for.

For hi2isis_k the extra magic it does is provide a default value for TO based on FROM if a parameter for TO is not specified, and so on.

The idea is that the _k functions provide more features, or program-specific features for how users might expect to use them than are available normally. I just made a few initial ones, to explore the space, and see if any patterns emerged (the default TO behavior in hi2isis_k might be generalizable, and other things may be too.

@jessemapel
Copy link
Contributor Author

jessemapel commented Feb 25, 2019

I had originally thought about implementing the second option, but it's got one issue. The "required" parameters for applications are intermingled with the "optional" parameters in the xml files. If the "required" arguments are positional arguments and the 'optional" arguments are kwargs it would make for an cleaner signatures.

Here's what I envision for spiceinit

import kalasiris.spiceinit as spiceinit
my_cube = 'some_file.cub'
spiceinit(my_cube)

Then, you could optionally tack some kwargs onto it for the shapemodel

import kalasiris.spiceinit as spiceinit
my_cube = 'some_file.cub'
my_dem = 'some_dem.cub'
spiceinit(my_cube, shape='user', model=my_dem)

If we just convert all of the application arguments into positional arguments, then you would have to enter every argument. So, your spiceinit signature would be spiceinit(from_, web, attach, cksmithed, ckrecon, ckpredict, cknadir, spksmithed, spkrecon, spkpredict, ls ,pck, tspk, ik, sclk, ck, fk, spk, iak, extra, shape, model, startpad, endpad, url, port).

@rbeyer
Copy link
Owner

rbeyer commented Feb 25, 2019

As far as I know, the two code snippets above should already work with kalasiris! :MindBlown:

So on the command line you can say:

isis3 % spiceinit from=some_file.cub shape=user model=some_dem.cub

and kalasiris supports the pysis calling style (assuming your import and variable inputs as above):

spiceinit(from_=my_cube, shape='user', model='my_dem')

But, like you, I felt that since almost all ISIS programs require a FROM= parameter, that should 'just work'™, so you can get the same effect as above, by doing exactly what you said:

spiceinit(my_cube, shape='user', model='my_dem')
spiceinit(my_cube)

If kalasiris gets one positional argument, it assumes it is a FROM= parameter, decorates it accordingly, and passes it on to kalasiris._run_isis_program() which just takes a list. And then it runs subprocess.call() with that list.

The call to kalasiris._run_isis_program() only passes on what it gets from the Factory-built ISIS function (made by _build_isis_fn()), so if you don't specify shape= or model= then those two don't get passed to kalasiris._run_isis_program() and they aren't in the subprocess.call() and therefore the ISIS program doesn't see them in its argument signature, and it defaults them, just like it would if you were typing at the command line.

Of course, this also means that you can do this:

spiceinit(my_cube, jesse='Awesome!')

which subprocess.call() would dutifully pass to spiceinit which would be the equivalent of this on the command line:

isis3 % spiceinit fr= some_file.cub jesse=awesome
**USER ERROR** Invalid command line.
**USER ERROR** Unknown parameter [jesse].

If you tried to do that in your Python, calling the above function would throw a subprocess.CalledProcessError that you can either be prepared for with a try-block or will crater your program (which it should).

And, of course, just for yuks, kalasiris also supports ISIS's 'reserved parameters' on any program, by using two underbars (_) after the name, because sometimes I find myself running spiceinit like this if I have a suite of custom SPICE kernels I want to load:

isis3 % spiceinit from=some_file.cub -restore=some.par

and you can do that with kalasiris like this:

spiceinit(my_cube, restore__='some.par')

There are two underbars after restore on that line.

I thought you were saying that you wanted the ability to have more than one regular positional argument (because, of course, you can already say spiceinit(my_cube, help__)). Such that the the other (non-reserved) positional arguments would just get auto-assigned to the ISIS parameters in the order they're listed in the docs. Sounds like you're maybe additionally saying that you'd want them auto-assigned to the 'required' parameters first, then the optional ones.

Any of that is possible, but would require more logic in the Factory-built function to read the XML, etc. It would be fun, but I'm tempted to defer it (and keep the code small in _build_isis_fn()) until there is a groundswell for it.

@jessemapel
Copy link
Contributor Author

Maybe this has less use that I thought if it only applies to 2+ argument calls. Something like k_funcs for all of the _2isis apps would be nice, but is another topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants