Document AgentType's model variable fields

[sbenthall]

In tracking the relationship between HARK.AgentType and the generic MDP formalism (see #491), I'm seeing some architectural choices made that, in my opinion, make it a little harder than necessary to follow the documentation.

At the class level documentation for AgentType, it says:

Critically, every subclass of AgentType should define class-specific static values of the attributes time_vary and time_inv as lists of strings. Each element of time_vary is the name of a field in AgentSubType that varies over time in the model. Each element of time_inv is the name of a field in AgentSubType that is constant over time in the model

time_vary and time_inv are not defined as variables on the AgentType class.
They easily could be (as empty lists), and then they could be documented individually as parameters.

Some other fields associated with "model variables", like poststate_vars and track_vars, are already like this (initialized to empty lists), but for some reason are not documented in the docstring.

Another variable shock_vars, is used but not documented at all.

I'd like to make these variables more explicit in the code and document them.

[sbenthall]

StackOverflow, referring to PEP257, on how to document class attributes, which while show up in Sphinx autodocs:
https://stackoverflow.com/questions/3051241/how-to-document-class-attributes-in-python

[sbenthall]

@mnwhite I'm noticing that when these time_vary attributes are filled in on subclasses of AgentType, you sometimes start with a variable with a trailing underscore after the variable name, e.g.:

https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L1595-L1598

And then later in the initializer assign it to the class:
https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L1632-L1635

I was wondering if there's any particular reason why you implemented it this way. This use of an underscore is as far as I can tell not of any relevant standard meaning in Python:
https://hackernoon.com/understanding-the-underscore-of-python-309d1a029edc

[mnwhite]

It's because I quickly realized that time_vary is *not* a class variable, but is, in fact, instance-specific. The underscored versions in the class definition are the baseline values-- what *has* to exist in an instance when it is created for the type to work properly. During initialization (usually what happens when the update() method is called at the end of init), several attributes are constructed from the primitives that were passed, and their names get added to time_vary once they're complete. The docstring that says they're class-specific is very outdated. The underscore was used because it's unobtrusive, no particular reason.

…

On Fri, Feb 7, 2020 at 9:48 AM Sebastian Benthall ***@***.***> wrote: @mnwhite <https://github.com/mnwhite> I'm noticing that when these time_vary attributes are filled in on subclasses of AgentType, you sometimes start with a variable with a trailing underscore after the variable name, e.g.: https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L1595-L1598 And then later in the initializer assign it to the class: https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsIndShockModel.py#L1632-L1635 I was wondering if there's any particular reason why you implemented it this way. This use of an underscore is as far as I can tell not of any relevant standard meaning in Python: https://hackernoon.com/understanding-the-underscore-of-python-309d1a029edc — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#492?email_source=notifications&email_token=ADKRAFIT7UT6ZZSVYXBTW23RBVYERA5CNFSM4KRGODJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELDFDAA#issuecomment-583422336>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADKRAFKOEQO6OI65KISBK73RBVYERANCNFSM4KRGODJQ> .

[sbenthall]

I see.

I see that update() is not defined on AgentType, but it is in the subclasses.

I would like to:

• make these model variable fields into parameters passed to the AgentType initializer
• document them
• have the AgentType initializer call a (dummy, but documented) update()
• have downstream classes pass their default model variable values into their superclass constructor,
• remove the calls to update() from the subclass constructors.

The principle here is just:

• have the critical code documented
• push the functionality to the level of abstraction in the class hierarchy where it generalizes best.

What do you think about this plan?

[mnwhite]

Removing the calls to update() from constructor methods might be tricky, if there's some other method that needs to be run first before update(). And I'm not sure I understand what the first bullet point item means.

…

On Sat, Feb 8, 2020 at 11:06 AM Sebastian Benthall ***@***.***> wrote: I see. I see that update() is not defined on AgentType, but it is in the subclasses. I would like to: - make these model variable fields into parameters passed to the AgentType initializer - document them - have the AgentType initializer call a (dummy, but documented) update() - have downstream classes pass their default values into their superclass constructor, - remove the calls to update() from the subclass constructors. The principle here is just: - have the critical code documented - push the functionality to the level of abstraction in the class hierarchy where it generalizes best. What do you think about this plan? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#492?email_source=notifications&email_token=ADKRAFITB72M76GUZUGZQ5DRB3J7XA5CNFSM4KRGODJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELFVDZY#issuecomment-583750119>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADKRAFP7NUOPHUUVYHO4KT3RB3J7XANCNFSM4KRGODJQ> .