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

Flows to functions #112

Merged
merged 4 commits into from
Nov 1, 2023
Merged

Flows to functions #112

merged 4 commits into from
Nov 1, 2023

Conversation

danielward27
Copy link
Owner

Breaking change:

  • All the flows have moved from being classes (with standard PascalCase naming) to functions with the standard snake_case naming.
  • Flow arguments other than the first argument (key) are now key-word only.

Other changes:

  • Using properties where possible, in order to avoid cluttering PyTrees with duplicated information.

Reason for the breaking change:

  • None of the flows implemented any methods or extended the base class AbstractTransformed in any way, so the use of classes is not needed. Instead, flows now are functions that return a Transformed distribution. By being a class, it implicitly suggests the flows must implement some methods or extend the base class, which is confusing.
  • The only advantage of having flows as classes was to allow storing of additional attributes in the class. In general this wasn't particularly useful, and most of the attributes provided duplicate information from the underlying bijection.
  • Enforcing key word only arguments for flows ensures readable code.

@danielward27 danielward27 merged commit 67d72e6 into main Nov 1, 2023
1 check passed
@danielward27 danielward27 deleted the properties branch November 1, 2023 12:57
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