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

Accessing unset f_args and jac_args attributes of ivp.IVP class should return None #77

Closed
davidrpugh opened this issue Sep 9, 2014 · 1 comment

Comments

@davidrpugh
Copy link
Contributor

Current behavior raises an AttributeError. This is undesirable. I think fix, just requires changing the setter for the attributes as follows...

def f_args(self, new_f_args):
        """Set new values for the parameters."""
        if not isinstance(new_f_args, (tuple, type(None))):
            mesg = "IVP.args must be a tuple, not a {}."
            raise AttributeError(mesg.format(new_f_args.__class__))
        elif new_f_args is not None:
            self.set_f_params(*new_f_args)
            self._f_args = new_f_args
        else:
            pass

to

def f_args(self, new_f_args):
        """Set new values for the parameters."""
        if not isinstance(new_f_args, (tuple, type(None))):
            mesg = "IVP.args must be a tuple, not a {}."
            raise AttributeError(mesg.format(new_f_args.__class__))
        else:
            self.set_f_params(*new_f_args)
            self._f_args = new_f_args

Similar fix for the jac_args setter. PR for this will follow shortly.

@davidrpugh
Copy link
Contributor Author

Actually, this issue has made me realize that I made a "poor" design decision in the way that I allow users to set arguments for the functions f and jac.

Currently there are two attributes of the ivp.IVP class that are serving this purpose: f_args and f_params (also jac_args and jac_params). I introduced the new f_args and jac_args attributes in order to conduct explicit type checking (something which scipy.integrate.ode should be doing but isn't).

However, I now think that the existence of these different attributes is likely to confuse users and I would like to remove both the f_args and jac_args attributes. This will require changes to the class, tests, and example notebook.

Thoughts?

davidrpugh added a commit to davidrpugh/quant-econ that referenced this issue Sep 9, 2014
jstac added a commit that referenced this issue Sep 9, 2014
@sglyon sglyon closed this as completed Sep 10, 2014
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