-
Notifications
You must be signed in to change notification settings - Fork 14
feat: Improve BlueapiClient to add plan parameter type hints #1469
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
base: main
Are you sure you want to change the base?
Changes from all commits
1467fc6
d67adf8
ff42043
7345817
689245f
e39a961
ec97173
a1ba5d4
a8aa805
17e86b0
48c79e0
3ce0eff
47f036f
bd6ceae
afd9841
0d0c85e
951eb7f
6272b42
ac57232
412b8b8
e694d46
fc13c02
8760ac6
4f34f4c
9938f2c
8a90f92
911c76f
2ad6299
0d777cb
a163bc5
5e8ee5b
1284e29
3728122
2e8531a
5a8212d
5a3bee9
5144d1b
2bc4f7b
ff36ef8
1de6f06
ee277c4
3e2dbf8
a61201f
ef4b821
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |||||||
| from concurrent.futures import Future | ||||||||
| from contextlib import suppress | ||||||||
| from functools import cached_property | ||||||||
| from itertools import chain | ||||||||
| from pathlib import Path | ||||||||
| from typing import Any, Self | ||||||||
|
|
||||||||
|
|
@@ -56,6 +55,16 @@ | |||||||
|
|
||||||||
| log = logging.getLogger(__name__) | ||||||||
|
|
||||||||
| _REPR_MAX_LENGTH = 100 | ||||||||
| _REPR_MAX_ARGS_INLINE = 3 | ||||||||
| _JSON_TYPE_MAP = { | ||||||||
| "string": "str", | ||||||||
| "integer": "int", | ||||||||
| "boolean": "bool", | ||||||||
| "number": "float", | ||||||||
| "object": "dict", | ||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
| class MissingInstrumentSessionError(Exception): | ||||||||
| pass | ||||||||
|
|
@@ -164,8 +173,8 @@ def help_text(self) -> str: | |||||||
| return self.model.description or f"Plan {self!r}" | ||||||||
|
|
||||||||
| @property | ||||||||
| def properties(self) -> set[str]: | ||||||||
| return self.model.parameter_schema.get("properties", {}).keys() | ||||||||
| def properties(self) -> dict[str, Any]: | ||||||||
| return self.model.parameter_schema.get("properties", {}) | ||||||||
|
|
||||||||
| @property | ||||||||
| def required(self) -> list[str]: | ||||||||
|
|
@@ -201,10 +210,30 @@ def _build_args(self, *args, **kwargs): | |||||||
| raise TypeError(f"Missing argument(s) for {missing}") | ||||||||
| return params | ||||||||
|
|
||||||||
| def __repr__(self): | ||||||||
| opts = [p for p in self.properties if p not in self.required] | ||||||||
| params = ", ".join(chain(self.required, (f"{opt}=None" for opt in opts))) | ||||||||
| return f"{self.name}({params})" | ||||||||
| def __repr__(self) -> str: | ||||||||
| required = set(self.required) | ||||||||
|
|
||||||||
| def _format_arg(name: str, info: dict[str, Any]) -> str: | ||||||||
| typ = _pretty_type(info) | ||||||||
| default = info.get("default") | ||||||||
|
|
||||||||
| if name in required: | ||||||||
| return f"{name}: {typ}" | ||||||||
| if default := info.get("default"): | ||||||||
| return f"{name}: {typ} = {default!r}" | ||||||||
| return f"{name}: {typ} | None = None" | ||||||||
|
|
||||||||
| props: dict = self.model.parameter_schema.get("properties", {}) | ||||||||
| args = [_format_arg(name, info) for name, info in props.items()] | ||||||||
|
Comment on lines
+226
to
+227
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| single_line = f"{self.name}({', '.join(args)})" | ||||||||
|
|
||||||||
| if len(single_line) <= _REPR_MAX_LENGTH and len(args) <= _REPR_MAX_ARGS_INLINE: | ||||||||
| return single_line | ||||||||
|
|
||||||||
| indent = " " | ||||||||
| # Fall back to multiline if too many arguments or too long. | ||||||||
| multiline_args = ",\n".join(f"{indent}{arg}" for arg in args) | ||||||||
| return f"{self.name}(\n{multiline_args}\n)" | ||||||||
|
|
||||||||
|
|
||||||||
| class BlueapiClient: | ||||||||
|
|
@@ -785,3 +814,22 @@ class PlanFailedError(Exception): | |||||||
| def __init__(self, typ: str, message: str): | ||||||||
| super().__init__(message) | ||||||||
| self._type = typ | ||||||||
|
|
||||||||
|
|
||||||||
| def _pretty_type(schema: dict[str, Any]) -> str: | ||||||||
| if "$ref" in schema: | ||||||||
| return schema["$ref"].split("/")[-1] | ||||||||
|
|
||||||||
| if schema.get("type") == "array": | ||||||||
| item_schema = schema.get("items", {}) | ||||||||
| inner = _pretty_type(item_schema) | ||||||||
| return f"list[{inner}]" | ||||||||
|
|
||||||||
| if "anyOf" in schema: | ||||||||
| return " | ".join(_pretty_type(s) for s in schema["anyOf"]) | ||||||||
|
|
||||||||
| json_type = schema.get("type") | ||||||||
| if isinstance(json_type, str): | ||||||||
| return _JSON_TYPE_MAP.get(json_type, json_type.split(".")[-1]) | ||||||||
|
|
||||||||
| return "Any" | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we want to be careful that we're not making an invalid schema. With these changes the schema contains several fields such as which in turn causes your repr to contain parameters such as |
Uh oh!
There was an error while loading. Please reload this page.