Parser: field classes defined in the project itself are not recognised

Open
  • Is something wrong?
#254 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
django, python
Domain
cli

Research direction

Start in cli/orm_lens/parser.py: BARE_FIELD_TYPES and BARE_FIELD_RE (around lines 82 and 176) decide which bare calls count as fields, and _pull_imported_bases() (around line 935) already follows imports to the defining file. Agree the approach on the issue first. Done means the repro in the issue reports tax_rate, key_filter and groups correctly, a new test covers the two-hop subclass case, and the golden snapshots are updated.

Written by the indexing model from the issue text.

Description

enhancement help wanted python

Source: pretix — src/pretix/helpers/models.py:79 (NormalizedDecimalField), src/pretix/base/models/fields.py:31 (MultiStringField), src/pretix/helpers/countries.py:68 (FastCountryField); weblate — weblate/trans/fields.py:10 (RegexField), weblate/utils/fields.py:34 (UsernameField), weblate/auth/models.py:916 (GroupManyToManyField, used at line 1004)

What happens

Big projects wrap Django fields in their own subclasses:

# helpers/models.py
class NormalizedDecimalField(DecimalField): ...

# models.py
tax_rate = NormalizedDecimalField(max_digits=7, decimal_places=4)

A bare call is only a field when its name is in BARE_FIELD_TYPES, so all of these are dropped: from describe, er and cascade (a dropped relation hides its whole edge), and drift lists them as "migrated but no longer declared".

On pretix most of its ~45 drift entries are this (tax_rate, country, phone, restrict_to_status, ...). On weblate it's User.username, User.groups (a M2M subclass, so the relation is lost too), Component.language_regex/variant_regex/key_filter, Screenshot.image.

Repro

core/fields.py

from django.db import models
from django.db.models import DecimalField


class NormalizedDecimalField(DecimalField):
    pass


class RegexField(models.TextField):
    pass


class GroupManyToManyField(models.ManyToManyField):
    pass

shop/models.py

from django.contrib.auth.models import Group
from django.db import models

from core.fields import GroupManyToManyField, NormalizedDecimalField, RegexField


class Product(models.Model):
    name = models.CharField(max_length=100)
    tax_rate = NormalizedDecimalField(max_digits=7, decimal_places=4)
    key_filter = RegexField(blank=True)
    groups = GroupManyToManyField(Group, blank=True)

$ orm-lens describe shop.Product
| `name` | CharField | `max_length=100` |

Expected: tax_rate (DecimalField), key_filter (TextField) and groups as a ManyToManyField relation to Group.

Where

cli/orm_lens/parser.py

  • line 82 / 176, BARE_FIELD_TYPES and BARE_FIELD_RE;
  • line 935, _pull_imported_bases(): already follows a module's imports to the file that defines a name. That's the piece to reuse here.
Suggested fix

One possible shape:

  1. In the body loop, when a line looks like name = SomeName( but SomeName isn't whitelisted, remember it as a candidate instead of dropping it.
  2. After the scan, resolve each candidate through the module's imports (same machinery as _pull_imported_bases). If its class inherits, directly or a few hops up, from a known Django field, record the field with that base's type. If it's a ForeignKey/OneToOneField/ManyToManyField subclass, fill the relation data the usual way.
  3. Unresolvable names stay dropped, as today.

Third-party fields (PhoneNumberField, CountryField, I18nCharField, AutoOneToOneField from django-annoying) are a separate question: a small map of well-known ones is fine as a follow-up, not needed here.

This is the bigger one in the batch, hence help wanted. Comment on the issue with your plan first, happy to talk it through.

How to test
  • A new test file with the repro (two modules, resolved through the import).
  • A subclass two hops away (class A(models.CharField), class B(A)).
  • A name that isn't a field class (x = make_thing(...)) is still ignored.
  • Run the golden snapshots; saleor/zulip may gain fields, update them and list the changes in the PR.
  • cd cli && python -m pytest -q, ruff and mypy pass.

Difficulty: help wanted, most of a day.

Dominant language
Python
Stars
84
Forks
13
No merged PRs in 30d

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from FROWNINGdev/django-orm-lens

All issues in FROWNINGdev/django-orm-lens

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.