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

Added Tuple for message contents #11

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Ahmet-Kaplan
Copy link
Contributor

We may use tuple[T1, ..., Tn] to indicate a tuple type

We may use tuple[T1, ..., Tn] to indicate a tuple type
@TalIfargan
Copy link
Collaborator

Hi @Ahmet-Kaplan,

Thank you for suggesting this modification for type hinting!
In general, since we are willing to support python versions >= 3.9 we don't need to use typing module imported type annotations (see here for details https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html). So in this specific case I would replace all the imported collection hint types with the built-in supported type hints (Tuple[str, bool] --> tuple[str, bool], List[Message] --> list[Message], etc..).

See if you can fix this patch according to my suggestions and include changes to all the type annotations.

In addition, if you are willing to help us go through the repo and make sure type annotations are consistent throughout all the modules, it can be super helpful!

@TalIfargan TalIfargan self-requested a review June 5, 2024 09:37
Copy link
Collaborator

@TalIfargan TalIfargan left a comment

Choose a reason for hiding this comment

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

please see comments and change accordingly :)

@@ -5,7 +5,7 @@

from dataclasses import dataclass
from enum import Enum
from typing import NamedTuple, Optional, List
from typing import NamedTuple, Optional, List, Tuple
Copy link
Collaborator

Choose a reason for hiding this comment

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

we want to move to the built-in (python 3.9 +) collection type hints/annotations. So instead of importing them from typing, using the built-in type annotations.

@@ -128,7 +128,7 @@ def pretty_repr(self, number: Optional[int] = None, conversation_name: str = '',
s += colored_text(sep * TEXT_WIDTH, text_color)
return s

def get_content_after_hiding_incomplete_code(self) -> (str, bool):
def get_content_after_hiding_incomplete_code(self) -> Tuple[str, bool]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

replace Tuple[str, bool] with tuple[str, bool]

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