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: Local implicit modules for @guppy #105

Merged
merged 16 commits into from
Jan 18, 2024
Merged

Conversation

aborgna-q
Copy link
Collaborator

Makes @guppy define local modules by default. Fixes #101.

  • Removes _Guppy.set_module
  • Adds a _Guppy.compile that compiles and returns the local module.
  • Similarly, a _Guppy.take_module that returns the module without compiling it (and removes it from the local context).
  • Adds a @guppy(compile=True) option as a fall-back to the previous behaviour, where it directly compiles a Hugr.

Look at the first commit. The second is just noise updating the tests to use the compile flag.

I wonder if it'd be better to have a different decorator that compiles directly to hugrs instead of the compile flag.

Tentatively adds a `compiled` option for directly generating a Hugr.
@aborgna-q aborgna-q requested a review from mark-koch January 15, 2024 20:06
Copy link
Collaborator

@mark-koch mark-koch left a comment

Choose a reason for hiding this comment

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

Looks good 👍

I wonder if it'd be better to have a different decorator that compiles directly to hugrs instead of the compile flag.

I think this depends on whether we want direct compilation of functions to be a user facing feature? I would tend to say no, since we're really only using it to write tests. In that case, we could just make it a local decorator in the tests package

guppy/decorator.py Outdated Show resolved Hide resolved
guppy/decorator.py Outdated Show resolved Hide resolved
guppy/decorator.py Outdated Show resolved Hide resolved
guppy/decorator.py Outdated Show resolved Hide resolved
@aborgna-q
Copy link
Collaborator Author

we could just make it a local decorator in the tests package

Would it ever be useful to directly compile to a hugr in a jupyter notebook?
There's the problem that the user has no way to include modules before that, so it may be pretty useless.

@mark-koch
Copy link
Collaborator

Would it ever be useful to directly compile to a hugr in a jupyter notebook?

Do we even expect users to look at Hugrs? At least I think this would be an expert-level feature, so it would probably be fine to retrieve it from the module instead of making the decorator do it.

There's the problem that the user has no way to include modules before that, so it may be pretty useless.

Agreed. So I think we can go ahead an move this to the tests package?

@aborgna-q
Copy link
Collaborator Author

I removed the compile flag from the decorator and made it a @compile_guppy method in test.utils instead.

@aborgna-q aborgna-q requested a review from mark-koch January 18, 2024 14:04
Copy link
Collaborator

@mark-koch mark-koch left a comment

Choose a reason for hiding this comment

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

Thank you, LGTM 👍 Just some naming suggestions

guppylang/decorator.py Outdated Show resolved Hide resolved
guppylang/decorator.py Outdated Show resolved Hide resolved
guppylang/decorator.py Outdated Show resolved Hide resolved
guppylang/decorator.py Outdated Show resolved Hide resolved
guppylang/decorator.py Outdated Show resolved Hide resolved
tests/error/util.py Outdated Show resolved Hide resolved
@aborgna-q aborgna-q requested a review from mark-koch January 18, 2024 15:16
`hugr` uses `gtypes`, which uses `hugr.tys`
@aborgna-q aborgna-q merged commit f52a5de into main Jan 18, 2024
1 check passed
@aborgna-q aborgna-q deleted the feat/decorator-module branch January 18, 2024 15:52
mark-koch added a commit that referenced this pull request Apr 11, 2024
🤖 I have created a release *beep* *boop*
---


## 0.2.0 (2024-04-11)


### ⚠ BREAKING CHANGES

* Make `qubit` type lower case
([#165](#165))

### Features

* Local implicit modules for `@guppy`
([#105](#105))
([f52a5de](f52a5de))
* New type representation with parameters
([#174](#174))
([73e29f2](73e29f2))


### Bug Fixes

* Make ZZMax a dyadic operation
([#168](#168))
([152485f](152485f)),
closes [#154](#154)
* Stop exiting interpreter on error
([#140](#140))
([728e449](728e449))
* Use correct TK2 gate names
([#190](#190))
([df92642](df92642))


### Documentation

* add reference to runner to readme
([#129](#129))
([45c2bf0](45c2bf0))
* Add short description and simplify readme for pypi
([#136](#136))
([667bba3](667bba3))


### Code Refactoring

* Make `qubit` type lower case
([#165](#165))
([0a42097](0a42097))


### Continuous Integration

* Use `release-please bootstrap`'s default config
([#187](#187))
([72e666a](72e666a))

---
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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Mark Koch <[email protected]>
croyzor pushed a commit that referenced this pull request Apr 16, 2024
🤖 I have created a release *beep* *boop*
---


## 0.2.0 (2024-04-11)


### ⚠ BREAKING CHANGES

* Make `qubit` type lower case
([#165](#165))

### Features

* Local implicit modules for `@guppy`
([#105](#105))
([f52a5de](f52a5de))
* New type representation with parameters
([#174](#174))
([73e29f2](73e29f2))


### Bug Fixes

* Make ZZMax a dyadic operation
([#168](#168))
([152485f](152485f)),
closes [#154](#154)
* Stop exiting interpreter on error
([#140](#140))
([728e449](728e449))
* Use correct TK2 gate names
([#190](#190))
([df92642](df92642))


### Documentation

* add reference to runner to readme
([#129](#129))
([45c2bf0](45c2bf0))
* Add short description and simplify readme for pypi
([#136](#136))
([667bba3](667bba3))


### Code Refactoring

* Make `qubit` type lower case
([#165](#165))
([0a42097](0a42097))


### Continuous Integration

* Use `release-please bootstrap`'s default config
([#187](#187))
([72e666a](72e666a))

---
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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Mark Koch <[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.

guppy.decorator.guppy.set_module breaks every test
2 participants