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: Unpacking assignment of iterable types with static size #688

Merged
merged 13 commits into from
Dec 17, 2024

Conversation

mark-koch
Copy link
Collaborator

@mark-koch mark-koch commented Dec 4, 2024

Closes #649.

Sorry for the big PR, but there was no good way to split the checking logic up. I recommend reviewing commit by commit, most of the actual logic is in 616c0c5. Hugr lowering follows in a separate PR (#689).

So we can reuse the comprehension logic to handle iterable unpacking
So we can reuse the comprehension logic to handle iterable unpacking
Drive-by: Use singledispatch to handle the different kinds of assignments
Since we print them in one of the new error messages
@mark-koch mark-koch requested a review from a team as a code owner December 4, 2024 15:17
@mark-koch mark-koch requested a review from aborgna-q December 4, 2024 15:17
Comment on lines 56 to 59
elif isinstance(lhs, TupleUnpack):
types = [get_type(e).to_hugr() for e in lhs.pattern.left]
unpack = self.builder.add_op(ops.UnpackTuple(types), port)
for pat, wire in zip(lhs.elts, unpack, strict=True):
for pat, wire in zip(lhs.pattern.left, unpack, strict=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.

This is just a temporary fix to make compilation for the existing tests work. It will be replaced with the updated Hugr lowering

Singledispatch on unions was only introduced in Python 3.11
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 98.49624% with 4 lines in your changes missing coverage. Please review.

Project coverage is 92.80%. Comparing base (a099d1a) to head (e2afb20).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
guppylang/checker/stmt_checker.py 98.38% 2 Missing ⚠️
guppylang/cfg/builder.py 95.00% 1 Missing ⚠️
guppylang/compiler/stmt_compiler.py 98.21% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #688      +/-   ##
==========================================
+ Coverage   92.62%   92.80%   +0.17%     
==========================================
  Files          71       71              
  Lines        7955     8140     +185     
==========================================
+ Hits         7368     7554     +186     
+ Misses        587      586       -1     

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

Comment on lines 206 to 207
at_least = "at least " if self.at_least else ""
return f"{msg} (expected {self.expected}, got {at_least}{self.actual})"
Copy link
Collaborator

Choose a reason for hiding this comment

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

The at_least message could probably go inside the diff < 0 branch for clarity, as it should always be false otherwise.

Comment on lines +101 to +102
@functools.singledispatchmethod
def _check_assign(self, lhs: ast.expr, rhs: ast.expr, rhs_ty: Type) -> ast.expr:
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice

4 | @compile_guppy
5 | def foo() -> int:
6 | a, *bs = 1, 2, True
| ^^ Expression of type `(int, int, bool)` cannot be collected
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this error only list the (int, bool) types?

[q4, *qs] = qs
*qs, q5, q6 = qs
[*qs] = qs
return q1, q2, q3, q4, q5, q6, qs
Copy link
Collaborator

Choose a reason for hiding this comment

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

q: Is there an issue for supporting the rhs case of unpacking, i.e. writing something like this?

return q1, q2, q3, q4, q5, q6, *qs

(currently it panics with a NotImplementedError)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created #690 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, we shouldn't be throwing NotImplementedError 😅 See #691

@mark-koch mark-koch enabled auto-merge December 17, 2024 17:05
@mark-koch mark-koch added this pull request to the merge queue Dec 17, 2024
Merged via the queue into main with commit 602e243 Dec 17, 2024
3 checks passed
@mark-koch mark-koch deleted the feat/unpacking branch December 17, 2024 17:11
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
🤖 I have created a release *beep* *boop*
---


## [0.14.0](v0.13.1...v0.14.0)
(2024-12-19)


### ⚠ BREAKING CHANGES

* Lists in `py(...)` expressions are now turned into Guppy arrays
instead of lists.
* `dirty_qubit` function removed
* measure_return renamed to `project_z`

### Features

* add `maybe_qubit` stdlib function
([#705](#705))
([a49f70e](a49f70e)),
closes [#627](#627)
* add measure_array and discard_array quantum function
([#710](#710))
([3ad49ff](3ad49ff))
* Add method to load pytket circuit without function stub
([#712](#712))
([ee1e3de](ee1e3de))
* Add Option type to standard library
([#696](#696))
([45ea6b7](45ea6b7))
* Allow generic nat args in statically sized ranges
([#706](#706))
([f441bb8](f441bb8)),
closes [#663](#663)
* Array comprehension
([#613](#613))
([fdc0526](fdc0526)),
closes [#614](#614)
[#616](#616)
[#612](#612)
* Implicit coercion of numeric types
([#702](#702))
([df4745b](df4745b)),
closes [#701](#701)
* Load `pytket` circuit as a function definition
([#672](#672))
([b21b7e1](b21b7e1))
* Make arrays iterable
([#632](#632))
([07b9871](07b9871))
* qsystem std functions with updated primitives
([#679](#679))
([b0f041f](b0f041f))
* remove dirty_qubit
([#698](#698))
([78e366b](78e366b))
* Turn py expression lists into arrays
([#697](#697))
([d52a00a](d52a00a))
* Unpacking assignment of iterable types with static size
([#688](#688))
([602e243](602e243))
* update to hugr 0.10 and tket2 0.6
([#725](#725))
([63ea7a7](63ea7a7))


### Bug Fixes

* Accept non-negative int literals and py expressions as nats
([#708](#708))
([a93d4fe](a93d4fe)),
closes [#704](#704)
* Allow borrowing inside comprehensions
([#723](#723))
([02b6ab0](02b6ab0)),
closes [#719](#719)
* Detect unsupported default arguments
([#659](#659))
([94ac7e3](94ac7e3)),
closes [#658](#658)
* docs build command
([#729](#729))
([471b74c](471b74c))
* Ensure `int`s can be treated as booleans
([#709](#709))
([6ef6d60](6ef6d60)),
closes [#681](#681)
* Fix array execution bugs
([#731](#731))
([0f6ceaa](0f6ceaa))
* Fix implicit modules in IPython shells
([#662](#662))
([4ecb5f2](4ecb5f2)),
closes [#661](#661)
* Properly report error for unsupported constants
([#724](#724))
([d0c2da4](d0c2da4)),
closes [#721](#721)
* Properly report errors for unsupported expressions
([#692](#692))
([7f24264](7f24264)),
closes [#691](#691)
* remove use of deprecated Ellipsis
([#699](#699))
([b819a84](b819a84))


### Documentation

* Fix docs build ([#700](#700))
([684f485](684f485)),
closes [#680](#680)
* fix README.md and quickstart.md
([#654](#654))
([abb0221](abb0221))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Seyon Sivarajah <[email protected]>
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.

Known-size array unpacking
3 participants