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

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

Open
DavidLitwin opened this issue Jul 30, 2020 · 2 comments
Open

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

DavidLitwin opened this issue Jul 30, 2020 · 2 comments

Comments

@DavidLitwin
Copy link

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
Copy link
Member

@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
Copy link
Author

@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 Add recharge to gdp callback landlab/landlab#1223 to discuss this more if necessary.

@DavidLitwin DavidLitwin changed the title Checking function inputs (Landlab#1223) Checking function inputs (landlab/landlab#1223) Jul 31, 2020
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