Skip to content

Decouple process class from Model #63

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

Merged
merged 8 commits into from
Sep 30, 2019
Merged

Decouple process class from Model #63

merged 8 commits into from
Sep 30, 2019

Conversation

benbovy
Copy link
Member

@benbovy benbovy commented Sep 29, 2019

This is a follow-up on #59 towards a less invasive framework.

@process is now a thin wrapper around attr.s. Applied on a class A, it returns the class (almost) just as it was decorated with attr.s. The more invasive modifications (custom properties, etc.) are now in a subclass of A that is created programmatically and accessible from A.__xsimlab_cls__ (Model objects use the latter subclass instead of A directly).

The great advantage is that it is now possible to use instances of process-decorated classes independently of any Model, which is IMO a better solution than the approach proposed in #50 for testing the logic implemented in those classes.

Variables declared with intent='in' or intent='inout' are now included in__init__ generated by attr.

An alternative approach would be to stick with one single class and customize its __init__ to circumvent the limitation of read-only properties created for input variables (see, e.g., python-attrs/attrs#393 (comment)). This is maybe less complicated than the approach used here, but it still alters the class significantly.

TODO:

This prevents issues when a default value is set for an input variable
that is declared before another input variable with no default (not
allowed by attr as it would mean positional argument after keyword
argument).
@benbovy benbovy merged commit 7f707ec into master Sep 30, 2019
@benbovy benbovy deleted the embed-process-cls branch December 11, 2019 08:56
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

Successfully merging this pull request may close these issues.

1 participant