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

IndShockConsumerType checkConditions() returns nothing, always #525

Closed
sbenthall opened this issue Feb 15, 2020 · 7 comments
Closed

IndShockConsumerType checkConditions() returns nothing, always #525

sbenthall opened this issue Feb 15, 2020 · 7 comments

Comments

@sbenthall
Copy link
Contributor

The checkConditions() method of IndShockConsumerType returns nothing whether or not the conditions it checks have been met.

Instead of returning, say, a boolean or an array of booleans corresponding to the checks, it prints statements to the user.

This is not ideal.
One reason it is not ideal is that it makes it difficult to write an automated test of this functionality--such as a test that an instance with a certain configuration would fail, or not, a condition.

See #504 #510

@mnwhite
Copy link
Contributor

mnwhite commented Feb 15, 2020 via email

@llorracc
Copy link
Collaborator

llorracc commented Feb 15, 2020 via email

@sbenthall
Copy link
Contributor Author

@mnwhite Ah, yes, I see. You're right. It does set the instance state.
I was mistaken; it is easier to write tests for this than I thought.

@sbenthall
Copy link
Contributor Author

@llorracc Something I'm finding is that the state that is set by the method appears to use a boolean, where True indicates that the condition is satisfied. That seems very natural to me. E.g.

if GIF<1:
self.GIC = True
if public_call:
print('The value of the growth impatience factor for the supplied parameter values satisfies the Growth Impatience Condition.', end = " ")
if verbose:
print('Therefore, a target level of wealth exists.')
print()
else:
self.GIC = False
violated = True
print('The given parameter values violate the Growth Impatience Condition for this consumer type; the GIF is: %2.4f' % (GIF), end = " ")
if verbose:
print('Therefore, a target level of wealth does not exist.')
print()

If the order does not matter but the conditions all have names, then it would make sense to return a dictioanry of booleans, where the keys are condition names and values are whether or not the condition was met.

@sbenthall
Copy link
Contributor Author

The variable violated seems to track whether or not any of the conditions have been violated. But I think nothing is done with that variable.

@sbenthall
Copy link
Contributor Author

Oh, interesting. The PerfForesightConsumerType class also has a checkConditions() method.
According to its docs, it returns None.
But according to the code, it returns a boolean based on whether or not any conditions were violated (True if any have been violated).
This is then passed the its subclass IndShcokConsumerType's checkConditions, which returns None (true to its documentation).

def checkConditions(self,verbose=False,verbose_reference=False,public_call=False):

The implemented type signatures of the superclass and subclass are different, though it stands to reason this should be the kind of thing with a clean inheritance structure with isomorphic type signatures.
The documentation is inaccurate on at least one of the classes.

There is room for improvement here.

@llorracc
Copy link
Collaborator

llorracc commented Feb 16, 2020 via email

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

No branches or pull requests

3 participants