Skip to content

Switch to Python 3.9+ type annotation syntax everywhere #539

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

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

Switch to Python 3.9+ type annotation syntax everywhere #539

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
62 changes: 25 additions & 37 deletions app/api/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Data models."""

from enum import Enum
from typing import Optional, Union
# remove this line completely
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

This looks like a temporary PR note that was accidentally committed into the source. Please remove this comment and, if needed, replace it with the actual intended change (i.e., delete the unused typing import line rather than leaving an instruction in code).

Suggested change
# remove this line completely

Copilot uses AI. Check for mistakes.

from fastapi.exceptions import HTTPException
from pydantic import (
Expand Down Expand Up @@ -44,28 +44,16 @@ class QueryModel(BaseModel):
# NOTE: Explicit examples are needed for fields requiring a URI to avoid random-string examples being generated
# for the example request body in the interactive docs

min_age: float = Field(default=None, ge=0)
max_age: float = Field(default=None, ge=0)
sex: str = Field(
default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"]
)
diagnosis: str = Field(
default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"]
)
min_num_imaging_sessions: int = Field(default=None, ge=0)
min_num_phenotypic_sessions: int = Field(default=None, ge=0)
assessment: str = Field(
default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"]
)
image_modal: str = Field(
default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"]
)
pipeline_name: str = Field(
default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"]
)
pipeline_version: str = Field(
default=None, pattern=VERSION_REGEX, examples=["1.0.0"]
)
min_age: float | None = Field(default=None, ge=0)
max_age: float | None = Field(default=None, ge=0)
sex: str | None = Field(default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"])
diagnosis: str | None = Field(default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"])
min_num_imaging_sessions: int | None = Field(default=None, ge=0)
min_num_phenotypic_sessions: int | None = Field(default=None, ge=0)
assessment: str | None = Field(default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"])
image_modal: str | None = Field(default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"])
pipeline_name: str | None = Field(default=None, pattern=CONTROLLED_TERM_REGEX, examples=["vocab:12345"])
pipeline_version: str | None = Field(default=None, pattern=VERSION_REGEX, examples=["1.0.0"])
Comment on lines +49 to +56
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

These lines are quite long and harder to scan/review. Consider formatting these Field(...) calls across multiple lines (as they were previously) or running the project formatter (e.g., Black) so arguments wrap consistently.

Copilot uses AI. Check for mistakes.

@model_validator(mode="after")
def check_maxage_ge_minage(self) -> Self:
Expand Down Expand Up @@ -107,13 +95,13 @@ class SessionResponse(BaseModel):
num_matching_phenotypic_sessions: int
num_matching_imaging_sessions: int
session_type: str
age: Optional[float]
sex: Optional[str]
age: float | None
sex: str | None
diagnosis: list
subject_group: Optional[str]
subject_group: str | None
assessment: list
image_modal: list
session_file_path: Optional[str]
session_file_path: str | None
completed_pipelines: dict


Expand All @@ -124,13 +112,13 @@ class CohortQueryResponse(BaseModel):

dataset_uuid: str
dataset_name: str
dataset_portal_uri: Optional[str]
dataset_portal_uri: str | None
dataset_total_subjects: int
records_protected: bool
num_matching_subjects: int
image_modals: list
available_pipelines: dict
subject_data: Union[list[SessionResponse], str]
subject_data: list[SessionResponse] | str


class DatasetQueryResponse(BaseModel):
Expand All @@ -140,14 +128,14 @@ class DatasetQueryResponse(BaseModel):
# dataset_file_path: str # TODO: Revisit this field once we have datasets without imaging info/sessions.
dataset_name: str
authors: list[str] = Field(default_factory=list)
homepage: Optional[str] = None
homepage: str | None = None
references_and_links: list[str] = Field(default_factory=list)
keywords: list[str] = Field(default_factory=list)
repository_url: Optional[str] = None
access_instructions: Optional[str] = None
access_type: Optional[str] = None
access_email: Optional[str] = None
access_link: Optional[str] = None
repository_url: str | None = None
access_instructions: str | None = None
access_type: str | None = None
access_email: str | None = None
access_link: str | None = None
dataset_total_subjects: int
records_protected: bool
num_matching_subjects: int
Expand All @@ -159,7 +147,7 @@ class SubjectsQueryResponse(BaseModel):
"""Data model for subject data matching a query."""

dataset_uuid: str
subject_data: Union[list[SessionResponse], str]
subject_data: list[SessionResponse] | str


class DataElementURI(str, Enum):
Expand All @@ -179,7 +167,7 @@ class StandardizedTermVocabularyNamespace(BaseModel):
vocabulary_name: str
namespace_url: str
namespace_prefix: str
version: Optional[str] # TODO: Make version mandatory?
version: str | None # TODO: Make version mandatory?
terms: list[dict]


Expand Down
22 changes: 11 additions & 11 deletions app/api/utility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import textwrap
from collections import namedtuple
from pathlib import Path
from typing import Any, Optional
from typing import Any

import httpx
import numpy as np
Expand Down Expand Up @@ -110,16 +110,16 @@ def create_bound_filter(var: str) -> str:

def create_query(
return_agg: bool,
age: Optional[tuple] = (None, None),
sex: Optional[str] = None,
diagnosis: Optional[str] = None,
min_num_imaging_sessions: Optional[int] = None,
min_num_phenotypic_sessions: Optional[int] = None,
assessment: Optional[str] = None,
image_modal: Optional[str] = None,
pipeline_name: Optional[str] = None,
pipeline_version: Optional[str] = None,
dataset_uuids: Optional[list] = None,
age: tuple | None = (None, None),
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

These annotations are very broad (tuple and list without element types), which weakens type checking and IDE support. Consider specifying element types (e.g., tuple[float | None, float | None] | None for age and list[str] | None for dataset_uuids, if those match the actual expected values).

Copilot uses AI. Check for mistakes.
sex: str | None = None,
diagnosis: str | None = None,
min_num_imaging_sessions: int | None = None,
min_num_phenotypic_sessions: int | None = None,
assessment: str | None = None,
image_modal: str | None = None,
pipeline_name: str | None = None,
pipeline_version: str | None = None,
dataset_uuids: list | None = None,
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

These annotations are very broad (tuple and list without element types), which weakens type checking and IDE support. Consider specifying element types (e.g., tuple[float | None, float | None] | None for age and list[str] | None for dataset_uuids, if those match the actual expected values).

Copilot uses AI. Check for mistakes.
) -> str:
"""
Creates a SPARQL query using a query template and filters it using the input parameters.
Expand Down
Loading