Skip to content

Fix objects.all() not coercing path fields to declared types (#147) - #501

Open
Yuren0312 wants to merge 1 commit into
jacebrowning:mainfrom
Yuren0312:fix-orm-path-field-coercion
Open

Yuren0312 wants to merge 1 commit into
jacebrowning:mainfrom
Yuren0312:fix-orm-path-field-coercion

Conversation

@Yuren0312

Copy link
Copy Markdown

Problem

Model.objects.all() yields instances whose path fields (e.g. the primary key
used in the datafile pattern) have the wrong type. For example:

@datafile("inventory/{self.pk}.yml")
class InventoryItem:
    pk: int
    name: str
    unit_price: float

InventoryItem.objects.get_or_create(42, "Things", 0.99)
items = list(InventoryItem.objects.all())
# items[0].pk == "42" (str), expected 42 (int)

This is tracked in #147 (the existing test_values_are_filled_from_disk test
was marked xfail).

Root cause

Manager.all() extracts placeholder values from the filename via parse(),
which returns strings. Unlike the other attributes, these path fields are not
stored in the file contents, so they never pass through the field's converter
in Mapper.load() — attrs deliberately excludes pattern fields (see
create_mapper()). The raw strings are then handed to Manager.get(), which
sets them on the instance unchanged.

Change

Coerce the path-derived values to their declared types before calling get(),
reusing the existing map_type() converter machinery (the same one
create_mapper() uses for regular attributes):

names = [name.removeprefix("self.") for name in result.named]
yield self.get(**dict(zip(names, self._coerce_values(names, values))))

_coerce_values() builds a {field.name: field} lookup from the dataclass's
init fields and runs each value through map_type(field.type).to_python_value().
Fields without a matching declaration (or without init) pass through
unchanged.

Verification

  • Removed the xfail marker from test_values_are_filled_from_disk; it now
    passes.
  • pytest tests/: 128 passed, 1 skipped, 4 xfailed (no regressions; the
    remaining xfails are unrelated pre-existing issues).
  • mypy, black --check, isort --check, pylint, and pydocstyle all pass.

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.

1 participant