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

fix: Minor fixes #217

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Minor fixes #217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#!/bin/sh

mkdir build

touch build/.nojekyll # Disable jekyll to keep files starting with underscores
Expand Down
12 changes: 4 additions & 8 deletions guppylang/definition/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,10 @@ class Definition(ABC):
name: str
defined_at: ast.AST | None

@property
@abstractmethod
def description(self) -> str:
"""Description of this definition to be used in messages to the user.

The returned text should fit into messages of the following form: "expected
a function, but got {description of this definition} instead".
"""
# Description of this definition to be used in messages to the user.
# The returned text should fit into messages of the following form: "expected
# a function, but got {description of this definition} instead".
description: str
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

N.B. in all of our subclasses of Definition, description is defined as a field

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does pyright complain about this? In the future we might have definitions where the description depends on the instance (for example if we unify struct and enum definitions into one class), so I would rather leave it more general

Copy link
Collaborator Author

@croyzor croyzor May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

To be clear: the complaint is that the parent has property called description, and the subclasses are shadowing that name with a str field. I think to inherit properties the subclasses also need to look like:

class A(Definition):
    @property
    def definition() -> str:
        ...

but I could easily be wrong

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. The thing is that fields on frozen dataclasses are observably equivalent to get-only properties, so it's the same interface. But it looks like pyright doesn't recognise this...

It's confusing that we use the same pattern for TypeBase.linear but it's not complaining there?



class ParsableDef(Definition):
Expand Down
2 changes: 1 addition & 1 deletion guppylang/definition/declaration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def synthesize_call(
return node, ty

def compile_outer(self, graph: Hugr, parent: Node) -> "CompiledFunctionDecl":
"""Adds a Hugr `FuncDecl` node for this funciton to the Hugr."""
"""Adds a Hugr `FuncDecl` node for this function to the Hugr."""
node = graph.add_declare(self.ty, parent, self.name)
return CompiledFunctionDecl(
self.id, self.name, self.defined_at, self.ty, self.python_func, node
Expand Down
2 changes: 1 addition & 1 deletion guppylang/tys/var.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from dataclasses import dataclass
from typing import ClassVar

# Type of de Bruijn indicies
# Type of de Bruijn indices
DeBruijn = int

# Type of unique variable identifiers
Expand Down
Loading