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: New type representation with parameters #174

Merged
merged 11 commits into from
Mar 19, 2024
Merged

Conversation

mark-koch
Copy link
Collaborator

@mark-koch mark-koch commented Mar 19, 2024

Most of the diff lines are just moving things to new files, so it's not as bad as it looks :D (see comments below)

These are the main new things:

  • Prepare support for types and functions that are generic over constant values (e.g. bounded nats):
    • Generic types and functions are now defined in terms of Parameters and Arguments that can be either types or constants (see tys/param.py and tys/arg.py).
    • Implementations for ConstParam and ConstArg will follow in the future
  • Improved pretty printing of types (see tys/printing.py)
  • Add a notion of TypeDefinition (see tys/definition.py) that replaces the ad-hoc creation of Python classes to define new types
  • BoolType is no longer a SumType. This was a hugr implementation detail and not relevant for the Guppy type system.

Drive by renaming:

  • Rename function type members from args/return to inputs/output since args is already used now
  • Rename GuppyType to Type

return UnknownFunctionType()
# If we don't have a specified type, then the extension writer has to
# provide their own type-checking code. Therefore, it doesn't matter which
# type we return here since it will never be inspected.
return FunctionType([], NoneType())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I got rid of UnknownFunctionType since it was very hacky. It's fine to return an arbitrary function type here since the actual checking logic is delegated to the extension writer

@@ -142,48 +142,9 @@ def type(

def dec(c: type) -> type:
_name = name or c.__name__

@dataclass(frozen=True)
class NewType(GuppyType):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Getting rid of this dynamic class creation is one of the big wins of this PR

Comment on lines -87 to -91
class UndefinedPort(OutPortV):
"""Dummy port for undefined variables.

Raises an `InternalGuppyError` if one tries to access one of its properties.
"""
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 was actually unused

guppylang/tys/parsing.py Outdated Show resolved Hide resolved
guppylang/tys/subst.py Outdated Show resolved Hide resolved
guppylang/tys/ty.py Outdated Show resolved Hide resolved
@mark-koch mark-koch requested a review from aborgna-q March 19, 2024 14:37
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.

Lots of changes, but LGTM 👍

"""Abstract base class for a type visitor that transforms types."""

@abstractmethod
def visit(self, arg: Any, /) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the positional-only arguments here, but not in the other ABCs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, they should be everywhere 👍

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 the "Improved pretty printing of types" look now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some additional parentheses in some cases to disambiguate, for example (T -> T) -> T was incorrectly printed as T -> T -> T before.

Also, if multiple type variables have the name chosen by the user, they now get disambiguated as T, T'1, T'2, ...

@mark-koch mark-koch merged commit 73e29f2 into main Mar 19, 2024
4 checks passed
@mark-koch mark-koch deleted the feat/type-defs branch March 19, 2024 17:26
This was referenced Apr 10, 2024
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.

2 participants