Skip to content

feat(simulators): make clear what is the default value of the sort argument in simulation runs list #2695

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
MortGron wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(simulators): make clear what is the default value of the sort argument in simulation runs list #2695

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
6 changes: 3 additions & 3 deletions cognite/client/_api/simulators/runs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ async def list(
model_revision_external_ids: SequenceNotStr[str] | None = None,
created_time: TimestampRange | None = None,
simulation_time: TimestampRange | None = None,
sort: SimulationRunsSort | None = None,
sort: SimulationRunsSort = SimulationRunsSort(),

@gemini-code-assist gemini-code-assist Bot Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Avoid using mutable or complex objects like SimulationRunsSort() as default arguments in function signatures. This is a well-known Python anti-pattern because default arguments are evaluated once at function definition time, which can lead to shared state bugs. Additionally, removing None from the allowed types of sort is a breaking change for any caller explicitly passing None (e.g., when forwarding arguments).

Please keep sort: SimulationRunsSort | None = None in the signature and handle the default value inside the function body.

Suggested change
sort: SimulationRunsSort = SimulationRunsSort(),
sort: SimulationRunsSort | None = None,
References
  1. Consistency: Follow established patterns across the codebase. Other list methods and call use sort: SomethingSort | None = None. (link)

@MortGron MortGron Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we wanted to enforce this rule, we could enable https://docs.astral.sh/ruff/rules/mutable-argument-default/#mutable-argument-default-b006. However, is not currently enabled. Also, the function does not mutate this argument, so there is no risk.

The change is breaking, but simulators is in alpha.

) -> SimulationRunList:
"""`Filter simulation runs <https://api-docs.cognite.com/20230101/tag/Simulation-Runs/operation/filter_simulation_runs_simulators_runs_list_post>`_

Expand All @@ -173,7 +173,7 @@ async def list(
model_revision_external_ids (SequenceNotStr[str] | None): Filter by model revision external ids
created_time (TimestampRange | None): Filter by created time
simulation_time (TimestampRange | None): Filter by simulation time
sort (SimulationRunsSort | None): The criteria to sort by.
sort (SimulationRunsSort): The criteria to sort by. Defaults to sorting by created time ascending.
Comment thread
MortGron marked this conversation as resolved.

Returns:
SimulationRunList: List of simulation runs
Expand Down Expand Up @@ -221,7 +221,7 @@ async def list(
resource_cls=SimulationRun,
list_cls=SimulationRunList,
filter=filter_runs.dump(),
sort=[SimulationRunsSort.load(sort).dump()] if sort else None,
sort=[SimulationRunsSort.load(sort).dump()],
Comment thread
MortGron marked this conversation as resolved.
)

@overload
Expand Down
6 changes: 3 additions & 3 deletions cognite/client/_sync_api/simulators/runs.py
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading