Repository navigation
minor update by agents - #305
Conversation
…hier into update/agents
for more information, see https://pre-commit.ci
…hier into update/agents
for more information, see https://pre-commit.ci
…hier into update/agents
|
Hey @Borda , I see this is not a draft.
Cheers! |
|
@shaypal5 can we roll back the deployment on PR if is not needed, since it makes PR super noisy and impossible to navigate in past comments... |
|
About the codecov, it reports too early, even before any test finishes, so for sure it will need to be fixed, but I would make it in a separate PR to keep the changes focused |
| return None | ||
|
|
||
| @staticmethod | ||
| def _get_raw_field(cached_data, field: str): |
There was a problem hiding this comment.
The static method _get_raw_field is missing a type annotation for the cached_data parameter. Based on the implementation, it should be typed as something like Dict[Union[bytes, str], Any] or a more specific type. The coding guidelines require "all new code must include full type annotations."
| return cached_data.get(field) | ||
|
|
||
| @staticmethod | ||
| def _get_bool_field(cached_data, name: str) -> bool: |
There was a problem hiding this comment.
The static method _get_bool_field is missing a type annotation for the cached_data parameter. It should be typed consistently with _get_raw_field. The coding guidelines require "all new code must include full type annotations."
| @staticmethod | ||
| def _loading_pickle(raw_value) -> Any: | ||
| """Load pickled data with some recovery attempts.""" | ||
| try: | ||
| if isinstance(raw_value, bytes): | ||
| return pickle.loads(raw_value) | ||
| elif isinstance(raw_value, str): | ||
| # try to recover by encoding; prefer utf-8 but fall | ||
| # back to latin-1 in case raw binary was coerced to str | ||
| try: | ||
| return pickle.loads(raw_value.encode("utf-8")) | ||
| except Exception: | ||
| return pickle.loads(raw_value.encode("latin-1")) | ||
| else: | ||
| # unexpected type; attempt pickle.loads directly | ||
| try: | ||
| return pickle.loads(raw_value) | ||
| except Exception: | ||
| return None | ||
| except Exception as exc: | ||
| warnings.warn( | ||
| f"Redis value deserialization failed: {exc}", | ||
| stacklevel=2, | ||
| ) | ||
| return None | ||
|
|
||
| @staticmethod | ||
| def _get_raw_field(cached_data, field: str): | ||
| """Fetch field from cached_data with bytes/str key handling.""" | ||
| # try bytes key first, then str key | ||
| bkey = field.encode("utf-8") | ||
| if bkey in cached_data: | ||
| return cached_data[bkey] | ||
| return cached_data.get(field) | ||
|
|
||
| @staticmethod | ||
| def _get_bool_field(cached_data, name: str) -> bool: | ||
| """Fetch boolean field from cached_data.""" | ||
| raw = _RedisCore._get_raw_field(cached_data, name) or b"false" | ||
| if isinstance(raw, bytes): | ||
| try: | ||
| s = raw.decode("utf-8") | ||
| except Exception: | ||
| s = raw.decode("latin-1", errors="ignore") | ||
| else: | ||
| s = str(raw) | ||
| return s.lower() == "true" |
There was a problem hiding this comment.
The docstrings for the new static methods (_loading_pickle, _get_raw_field, _get_bool_field) do not follow numpy docstring conventions as required by the coding guidelines. Numpy-style docstrings should include sections like Parameters, Returns, and potentially Notes or Examples. For example, _loading_pickle should document the raw_value parameter, describe what it returns, and explain the recovery attempts it makes.
| - id: ruff | ||
| name: Ruff check | ||
| args: ["--fix"] | ||
| args: ["--fix"] #, "--unsafe-fixes" |
There was a problem hiding this comment.
The commented-out --unsafe-fixes argument suggests that unsafe fixes were considered but not enabled. While commenting is fine for documentation, it would be clearer to either remove the comment entirely or add an explanatory comment about why unsafe fixes are not enabled. The current inline comment without explanation may cause confusion for future maintainers.
| args: ["--fix"] #, "--unsafe-fixes" | |
| args: ["--fix"] # Unsafe fixes are intentionally disabled in CI; add "--unsafe-fixes" locally if you accept the risk. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
…hier into update/agents
for more information, see https://pre-commit.ci
|
@shaypal5 seems all is green :) |
Ran some automatic issue fixing...