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.

Sign up for GitHub

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

Jump to bottom

fix: Accept non-negative int literals and py expressions as nats #708

Merged
merged 4 commits into from
Dec 12, 2024
Merged

fix: Accept non-negative int literals and py expressions as nats #708

merged 4 commits into from
Dec 12, 2024

Conversation

mark-koch
Copy link
Collaborator

Fixes #704

@mark-koch mark-koch requested a review from a team as a code owner December 10, 2024 18:11
@mark-koch mark-koch requested a review from croyzor December 10, 2024 18:11
mark-koch
mark-koch commented Dec 10, 2024
View reviewed changes
guppylang/std/builtins.py
Comment on lines +109 to 112
# TODO: Type information doesn't flow through the `if` expression, so we
# have to insert the `nat` coercions by hand.
# See https://github.com/CQCL/guppylang/issues/707
return nat(1) if self else nat(0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sadly, this still fails due to #707

mark-koch
mark-koch commented Dec 10, 2024
View reviewed changes
guppylang/checker/expr_checker.py
Comment on lines +1173 to +1175
return int_type()
case float():
return cast(TypeDef, globals["float"]).check_instantiate([], globals)
return float_type()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Drive-by: No need to look these up in globals

@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.66%. Comparing base (d52a00a) to head (5ad8fe6).

Files with missing lines Patch % Lines
guppylang/checker/expr_checker.py 94.73% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #708      +/-   ##
==========================================
+ Coverage   92.61%   92.66%   +0.05%     
==========================================
  Files          71       71              
  Lines        7882     7895      +13     
==========================================
+ Hits         7300     7316      +16     
+ Misses        582      579       -3

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

croyzor
croyzor reviewed Dec 11, 2024
View reviewed changes
guppylang/checker/expr_checker.py
"""Turns a primitive Python value into a Guppy type.

Accepts an optional `type_hint` for the expected expression type that is used to
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could add a comment explaining that this type hint is only used internally for coercing things to nat and it's an internal error to give a type hint to something that wouldnt otherwise synthesize int type

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently it's not an internal error, the hint is just ignored if it's invalid. The actual checking that the synthesised type matches what's expected happens in the ExprChecker.

A wider refactor of this would be good, but imo too much for this PR.

ss2165
ss2165 reviewed Dec 12, 2024
View reviewed changes
tests/error/py_errors/negative_nat.py

@compile_guppy
def foo() -> nat:
return py(-1)
Copy link
Member

Choose a reason for hiding this comment

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

could you add a test like range(py(4)) just so we can use it for regression checking

@mark-koch mark-koch added this pull request to the merge queue Dec 12, 2024
Merged via the queue into main with commit a93d4fe Dec 12, 2024
3 checks passed
@mark-koch mark-koch deleted the fix/nat-literals branch December 12, 2024 12:18
@hugrbot hugrbot mentioned this pull request Dec 12, 2024
Merged
@mark-koch mark-koch mentioned this pull request Dec 17, 2024
Closed
github-merge-queue bot pushed a commit that referenced this pull request Dec 19, 2024
@hugrbot @ss2165
chore(main): release 0.14.0 (#655)
db86f98 
🤖 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
Reviewers

@croyzor croyzor croyzor left review comments

@ss2165 ss2165 ss2165 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Non-negative literals should check against nat
4 participants
@mark-koch @codecov-commenter @croyzor @ss2165