-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: Generate constructor methods for structs #262
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #262 +/- ##
==========================================
+ Coverage 91.05% 91.36% +0.30%
==========================================
Files 44 44
Lines 4973 5000 +27
==========================================
+ Hits 4528 4568 +40
+ Misses 445 432 -13 ☔ View full report in Codecov by Sentry. |
guppylang/module.py
Outdated
if isinstance(defn, CheckedStructDef): | ||
for method_def in defn.generated_methods: | ||
generated[method_def.id] = method_def | ||
self._globals.impls.setdefault(defn.id, {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this wants to happen in the outer loop, so that the impls aren't set to {}
each time a method is added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setdefault
only sets if the key doesn't exist yet
https://docs.python.org/3/library/stdtypes.html#dict.setdefault
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I think this still wants to be in the outer loop, but is ultimately harmless then
guppylang/definition/struct.py
Outdated
|
||
constructor_sig = FunctionType( | ||
inputs=[f.ty for f in self.fields], | ||
output=StructType([p.to_bound(i) for i, p in enumerate(self.params)], self), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this work, I assume the list is the args
from ParametrizedTypeBase
, then self
is the defn
from StructType
. So in the default constructors for python dataclasses, you provide the fields for all of the parents, outer to inner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly. I could also use keyword args if it would be clearer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes please!
ty: FunctionType | ||
higher_order_value: bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this not true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the type of the function cannot be expressed in Guppy's type system.
For example, the len(x)
function is a CustomFunction
that checks if x
implements __len__
. We can't write down a type for len
, so we can't use it as a value
🤖 I have created a release *beep* *boop* --- ## [0.6.0](v0.5.2...v0.6.0) (2024-07-02) ### Features * Add array type ([#258](#258)) ([041c621](041c621)) * Add nat type ([#254](#254)) ([a461a9d](a461a9d)) * Add result function ([#271](#271)) ([792fb87](792fb87)), closes [#270](#270) * Allow constant nats as type args ([#255](#255)) ([d706735](d706735)) * Generate constructor methods for structs ([#262](#262)) ([f68d0af](f68d0af)), closes [#261](#261) * Return already-compiled hugrs from `GuppyModule.compile` ([#247](#247)) ([9d01eae](9d01eae)) * Turn int and float into core types ([#225](#225)) ([99217dc](99217dc)) ### Bug Fixes * Add missing test file ([#266](#266)) ([75231fe](75231fe)) * Loading custom polymorphic function defs as values ([#260](#260)) ([d15b2f5](d15b2f5)), closes [#259](#259) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Closes #261