csdms / help-desk

The CSDMS Help Desk. Ask questions. Get answers (about CSDMS products and services).
MIT License
6 stars 1 forks source link

Checking function inputs (landlab/landlab#1223) #33

Open DavidLitwin opened 4 years ago

DavidLitwin commented 4 years ago

I want to change the callback function in the GroundwaterDupuitPercolator to have the form: callback_function(grid, recharge_rate, substep_dt, **kwargs). Presently it has the form callback_function(grid, substep_dt, **kwargs). @kbarnhart suggested that the pr should include testing the number of arguments in the user-provided function. This seems tricky given that the user will likely provide additional keyword arguments (e.g. the folder and file location to write an output) that confuse most of the methods I've seen that count the number of arguments a function takes. Is there a simple solution to this that can correctly ensure three (or two, giving a depricated warning) non-keyword arguments are supplied?

mcflugen commented 4 years ago

@DavidLitwin The Pythonic way of doing it would be to try to call the supplied callback function and handling the error if the callback function doesn't have the correct number of arguments. Another solution would be to examine the signature of the callback using inspect.signature.

For the first method, your code would look something like,

try:
    callback_function(grid, recharge_rate, substep_dt, **kwargs)
except TypeError as error:
    # do something to handle the error

You have to be careful here though as you don't want to hide an unhandled TypeError that might have been generated within the callback function itself. To check this, you could look at the error string in error.args to see what caused the TypeError (the error would be something like 'callback_function() takes 2 positional arguments but 3 were given').

For the second method, your code might include something like,

parameters = inspect.signature(callback_function).parameters
n_args = 0
for p in parameters.values():
    if p.kind in (p. POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD) and p.default is p.empty):
        n_args += 1

There are however cases where the above wouldn't work. For example, if the signature of the callback looked like callback_function(*args).

I think the easiest, and most straightforward (and pythonic!) is probably the first method.

DavidLitwin commented 4 years ago

@mcflugen Thank you for your thoughts on this! I think I will go with the first option. I do have a few questions though.

  1. Say that the callback function writes something to a file. Does this mean that the file will always have some evidence of this test written to it, or is there a way to avoid that?
  2. Katy suggested that for now, I should make my changes such that the old format is still accepted, with a deprecation warning. It seems like this adds a level of complexity, because then I would have to know which method was used, and have options for calling each function. I could probably come up with something, but I'm curious if you have ideas, or if it be alright to just update? Not sure anyone is using this callback functionality except me at the moment. We can head over to landlab/landlab#1223 to discuss this more if necessary.