Opened 14 months ago
Closed 3 weeks ago
#36523 closed Cleanup/optimization (fixed)
Implement helper method to find module path of value
| Reported by: | Jake Howard | Owned by: | Vimal Sahani |
|---|---|---|---|
| Component: | Utilities | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Jake Howard | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Finding the module path for where a given class or function is defined is functionality used in a few places across the Django codebase. Whilst in the majority of cases, f"{val.__module__}.{val.__qualname__}" is enough, there are some cases it's not.
As part of #35859, a few more cases of this pattern are being added to the codebase. I think we're now at the point where it makes sense to extract this out to a helper function somewhere in django.utils. The implementation in django.migrations.serializer.TypeSerializer seems to be the most robust, however will want some more thorough testing based on different input types.
Change History (15)
comment:1 by , 14 months ago
comment:2 by , 14 months ago
| Component: | Uncategorized → Utilities |
|---|---|
| Easy pickings: | unset |
| Triage Stage: | Unreviewed → Someday/Maybe |
| Version: | → dev |
Thank you Jake for creating the ticket. Could please enumerate the locations of where this functionality is found?
comment:3 by , 14 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
Hi, I’m interested in working on this.
From what I understand, the goal is to extract the existing TypeSerializer logic for determining a module path into a reusable helper in django.utils, and ensure it covers edge cases where f"{val.module}.{val.qualname}" isn’t sufficient.
My plan would be:
Review and adapt the current implementation in django.migrations.serializer.TypeSerializer.
Add unit tests for different input types (functions, classes, nested classes, builtins, etc.).
Place the helper in django.utils and update the current usages in the codebase to use it.
Could you confirm if this approach makes sense, and if there’s a preferred name/location for the helper before I start a patch?
comment:4 by , 13 months ago
| Cc: | added |
|---|
That sounds like the right approach to me - thanks for picking this up!
comment:8 by , 3 weeks ago
| Description: | modified (diff) |
|---|
comment:9 by , 3 weeks ago
| Has patch: | set |
|---|
comment:10 by , 3 weeks ago
| Owner: | changed from to |
|---|
comment:11 by , 3 weeks ago
| Patch needs improvement: | set |
|---|
comment:12 by , 3 weeks ago
| Patch needs improvement: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
comment:13 by , 3 weeks ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Ready for checkin → Accepted |
comment:14 by , 3 weeks ago
| Needs tests: | unset |
|---|---|
| Triage Stage: | Accepted → Ready for checkin |
Probably worth holding off on this ticket until #35859 is merged, so all uses can be covered.