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

Initial template def and data fetch and post. #54

Merged
merged 11 commits into from
Jun 25, 2024
Merged

Initial template def and data fetch and post. #54

merged 11 commits into from
Jun 25, 2024

Conversation

mikelgg93
Copy link
Member

No description provided.

Copy link
Collaborator

@dourvaris dourvaris left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, have some code comments, also linting needs fixing.

@@ -146,6 +147,44 @@ async def send_event(
raise DeviceError(response.status, confirmation["message"])
return Event.from_dict(confirmation["result"])

async def get_template(self) -> Template:
"""
:raises
Copy link
Collaborator

Choose a reason for hiding this comment

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

Empty docstring?

return Template.fromdict(result)

async def get_template_data(self):
""" """
Copy link
Collaborator

Choose a reason for hiding this comment

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

Empty docstring?

@@ -3,6 +3,7 @@
import enum
import logging
import typing as T
import uuid
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer to use from module import thing as it shows issues at compile time rather than run time attribute errors ie:

from uuid import UUID
from datetime import datetime

def fromdict(cls, data: T.Dict) -> "Template":
items = [TemplateItem.fromdict(item) for item in data.get("items", [])]
return cls(
archived_at=datetime.fromisoformat(data["archived_at"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps this needs to be datetime.datetime.fromisoformat?

id: T.Optional[uuid.UUID] = None
input_type: str = dataclasses.field(default="any")
required: bool = dataclasses.field(default=False)
title: T.Optional[str] = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think some of these properties can actually be None, would be better to put Optional only on fields that can be None.

src/pupil_labs/realtime_api/models.py Outdated Show resolved Hide resolved
updated_at: T.Optional[datetime.datetime] = None

@classmethod
def fromdict(cls, data: T.Dict) -> "Template":
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something to consider in the future is something like pydantic and get all the validation/conversion for free, eg:

from uuid import UUID
from datetime import datetime
from typing import Literal
from pydantic.dataclasses import dataclass



@dataclass
class Template:
    id: UUID
    created_at: datetime
    items: list[TemplateItem]

WidgetType = Literal["TEXT", "PARAGRAPH", "RADIO_LIST", "CHECKBOX_LIST", "SECTION_HEADER", "PAGE_BREAK"]

@dataclass
class TemplateItem:
    widget_type: WidgetType
    title: str

json_data = {
    "id": "99ae566d-5988-4e47-8147-297d9ac989a4",
    "created_at": "2022-01-01T01:01:01",
    "items": [
        {"widget_type": "TEXT", "title": "a question"},
    ],
}

Template(**json_data)
# Template(id=UUID('99ae566d-5988-4e47-8147-297d9ac989a4'), created_at=datetime.datetime(2022, 1, 1, 1, 1, 1), items=[TemplateItem(widget_type='TEXT', title='a question')])

@dourvaris dourvaris merged commit e68e64c into pupil-labs:main Jun 25, 2024
16 checks passed
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