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

Move core language classes to frozen dataclasses? #22

Closed
shanest opened this issue Nov 30, 2023 · 3 comments
Closed

Move core language classes to frozen dataclasses? #22

shanest opened this issue Nov 30, 2023 · 3 comments
Assignees

Comments

@shanest
Copy link
Collaborator

shanest commented Nov 30, 2023

We ran into a lot of issues with mutability, hashing, and whatnot in the modals project. While they mostly seem to be resolved, I'm thinking it might be more elegant to re-factor our core classes into dataclasses, with their auto-generated hashers and whatnot.

@shanest
Copy link
Collaborator Author

shanest commented Dec 5, 2023

Major hurdle / issue to consider: Referent. Conceptually, this could really just be Referent = object. And then each application would specify a subclass of Referent that gives the actual fields (e.g. force and flavor for modals). On this approach, the main point of having Referent around is for conceptualization and readability.

The main use case of it right now is in Universe.from_dataframe, which is designed to enable folks to specify Referents and Universe in a CSV file. This method makes use of Referent.__init__ and its use of __dict__. This is a bit "frustrating", since a Referent is a paradigm for a frozen/immutable object.

Some options:

  1. Let from_csv and from_dataframe also take in a referent_type argument that is the constructor for the relevant subclass of Referent. This, however, requires users to define their own class (likely a dataclass), which is not as user-friendly as described above.
  2. Rewrite from_csv and from_dataframe to just call o.__dict__.update() directly.
  3. ...?

@shanest
Copy link
Collaborator Author

shanest commented Dec 5, 2023

Option 3 (where I'm leaning right now): keep Referent, including its flexible initializer, but then overwrite __setattr__ to imitate frozen-ness. I'll make a commit in the dataclass_refactor branch reflecting this idea soon.

@shanest
Copy link
Collaborator Author

shanest commented Feb 5, 2024

See PRs 23 (#23) and this commit: 6023cdc

@shanest shanest closed this as completed Feb 5, 2024
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