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

Replace DressedType with parameters on all the Types. #32

Merged
merged 54 commits into from
Dec 13, 2019

Conversation

jpivarski
Copy link
Member

Making DressedType a new Type object complicates the hierarchy (different structure than the bare type). Each Type has an (unused) parameters, which should instead be used to augment the existing Type objects in an unchanged hierarchy.

The wrapper class (currently the "dress") can become an entry in the parameters named __class__, and this leaves room for a Numba wrapper and higher-array wrapper classes.

The Dress and PyDress classes should go away, but the Parameters and PyParameters stay. All of the existing Types must now be templated on the Parameters type. Its parameters should probably become std::map<std::string, py::object> rather than py::dict so that the parameter names, at least, can be inspected on the C++ side. Because of Numba (#31), all of those Python objects need to be hashable and pickleable, so there's a natural serialization that can be performed on the C++ side (because PyParameters defines the picklization into bytestrings).

glass-ships and others added 30 commits December 5, 2019 20:59
…Type: need to change their signatures to accomodate.
…e does. (Get rid of FillableArray::type? It's redundant because you can always take a snapshot.)
@jpivarski jpivarski changed the title [WIP] Replace DressedType with parameters on all the Types. Replace DressedType with parameters on all the Types. Dec 13, 2019
@jpivarski jpivarski merged commit e6b2906 into master Dec 13, 2019
@jpivarski jpivarski deleted the feature/replace-dressedtype-with-parameters branch December 13, 2019 19:59
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.

2 participants