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

feat: Generate constructor methods for structs #262

Merged
merged 5 commits into from
Jun 25, 2024
Merged

Conversation

mark-koch
Copy link
Collaborator

Closes #261

@mark-koch mark-koch requested review from ss2165, a team and croyzor and removed request for ss2165 and a team June 24, 2024 12:37
Base automatically changed from fix/higher-order-poly to main June 25, 2024 10:07
@mark-koch mark-koch requested a review from a team as a code owner June 25, 2024 10:07
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.36%. Comparing base (d15b2f5) to head (e940574).

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.
📢 Have feedback on the report? Share it here.

if isinstance(defn, CheckedStructDef):
for method_def in defn.generated_methods:
generated[method_def.id] = method_def
self._globals.impls.setdefault(defn.id, {})
Copy link
Collaborator

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

Copy link
Collaborator Author

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

Copy link
Collaborator

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


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),
Copy link
Collaborator

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?

Copy link
Collaborator Author

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?

Copy link
Collaborator

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
Copy link
Collaborator

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?

Copy link
Collaborator Author

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

@mark-koch mark-koch enabled auto-merge June 25, 2024 15:27
@mark-koch mark-koch added this pull request to the merge queue Jun 25, 2024
Merged via the queue into main with commit f68d0af Jun 25, 2024
3 checks passed
@mark-koch mark-koch deleted the feat/constructors branch June 25, 2024 15:31
github-merge-queue bot pushed a commit that referenced this pull request Jul 2, 2024
🤖 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).
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.

Auto-generate struct constructor methods
3 participants