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

fix: Avro dataclass introspect typing #976

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

jjaakola-aiven
Copy link
Contributor

About this change - What it does

References: #xxxxx

Why this way

@jjaakola-aiven jjaakola-aiven requested review from a team as code owners October 16, 2024 10:47
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-fix-avro-dataclass-introspect-typing branch 2 times, most recently from 45215d3 to f736125 Compare October 16, 2024 11:16
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-fix-avro-dataclass-introspect-typing branch from f736125 to e54ad60 Compare October 16, 2024 11:18
@@ -128,7 +135,7 @@ def _field_type(field: Field, type_: object) -> AvroType: # pylint: disable=too
T = TypeVar("T", str, int, bool, Enum, None)


def transform_default(type_: type[T], default: T) -> str | int | bool | None:
def transform_default(type_: type[T] | str, default: T) -> str | int | bool | None:

Choose a reason for hiding this comment

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

What is the failure case that necessitates the addition of str here? Just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is pure typing correction. It looks that primitive types use string instead of type[T].

Copy link
Contributor

Choose a reason for hiding this comment

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

Tracking this to here:

https://github.com/python/typeshed/blob/6feca188689e14cdacd8dbf8bc9cf0635993027a/stdlib/dataclasses.pyi#L111

When originally authored, this type was type, only changed recently and I guess propagated in most recent mypy bump:

python/typeshed@6b2033a

Copy link
Contributor

Choose a reason for hiding this comment

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

_field_type doesn't give a type error because it accepts object and makes no assumptions. It's probably somewhat likely to break when the new deferred annotations arrive, but I wouldn't deal with that until it's possible to test with.

Copy link

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/karapace
  auth.py
  client.py
  src/karapace/avro_dataclasses
  introspect.py
  models.py
  src/karapace/backup
  api.py
  cli.py
  errors.py
  poll_timeout.py
  safe_writer.py
  src/karapace/backup/backends
  reader.py
  v1.py
  v2.py
  writer.py
  src/karapace/backup/backends/v3
  backend.py
  readers.py
  schema.py
  schema_tool.py
  writers.py
  src/karapace/compatibility
  __init__.py
  schema_compatibility.py
  src/karapace/compatibility/jsonschema
  checks.py
  types.py
  utils.py
  src/karapace/coordinator
  master_coordinator.py
  schema_coordinator.py
Project Total  

The report is truncated to 25 files out of 76. To see the full report, please visit the workflow summary page.

This report was generated by python-coverage-comment-action

Copy link
Contributor

@keejon keejon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@keejon keejon merged commit 458b1a6 into main Oct 16, 2024
9 checks passed
@keejon keejon deleted the jjaakola-aiven-fix-avro-dataclass-introspect-typing branch October 16, 2024 12:00
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.

4 participants