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: Add array type #258

Merged
merged 29 commits into from
Jun 25, 2024
Merged

feat: Add array type #258

merged 29 commits into from
Jun 25, 2024

Conversation

mark-koch
Copy link
Collaborator

No description provided.

@guppy.extend_type(builtins, array_type_def)
class Array:
@guppy.hugr_op(builtins, DummyOp("ArrayGet"))
def __getitem__(self: array[T, n], idx: int) -> T: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type varable T is declared as non-linear, so it cannot be instantiated with qubit (see error tests).

We first need some changes to the type checker in order to fully support linear arrays

13: def main(qs: array[qubit, 42]) -> int:
14: return qs[0]
^^
GuppyTypeError: Cannot instantiate non-linear type variable `T` in type `forall n, T: nat. (array[T, n], int) -> T` with linear type `qubit`
Copy link
Collaborator Author

@mark-koch mark-koch Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is not super helpful, but we only need it temporarily until we have proper linear arrays

@mark-koch mark-koch requested a review from aborgna-q June 24, 2024 10:05
Copy link
Collaborator

@aborgna-q aborgna-q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linear array errors look quite confusing, but I guess they are OK as temporary measure.

Comment on lines 66 to 67
class array(Generic[_S, _T]):
"""Class to import in order to use arrays."""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The names are switched from the Array definition below (which uses arrar[T, n]).
Can you document which typevar is the array type and which one the length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is just for the purpose of mypy and ruff to stop them from complaining, so it really doesn't matter what type vars we use here. But you're right, I'll choose some more suggestive names 👍

Base automatically changed from feat/nat-args to main June 25, 2024 08:08
@mark-koch mark-koch requested a review from a team as a code owner June 25, 2024 08:21
@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2024

Codecov Report

Attention: Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.93%. Comparing base (d706735) to head (b45899d).
Report is 1 commits behind head on main.

Files Patch % Lines
guppylang/prelude/_internal.py 84.21% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
+ Coverage   90.67%   90.93%   +0.26%     
==========================================
  Files          44       44              
  Lines        4954     4976      +22     
==========================================
+ Hits         4492     4525      +33     
+ Misses        462      451      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mark-koch mark-koch added this pull request to the merge queue Jun 25, 2024
Merged via the queue into main with commit 041c621 Jun 25, 2024
3 checks passed
@mark-koch mark-koch deleted the feat/array-ops branch June 25, 2024 08:48
github-merge-queue bot pushed a commit that referenced this pull request Jun 25, 2024
This file should have been added with #258
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.

3 participants