#24121 closed New feature (fixed)
Provide a better repr() experience
Reported by: | Keryn Knight | Owned by: | Nicolás Restrepo |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | django@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
For a long time, Django has shipped with the debug 500 handler, which defaults to a view where "local vars" for every stack frame can be expanded and inspected to see the internal state at the various points in the stack trace.
Between that renderer, the heavy usage of django-debug-toolbar
, which ships with a template context inspector, and my preference throwing pdb
everywhere, there is a notable lack of a good repr()
experience in much of Django, both in debugging public API (that is, the bits a user will use and encounter), and internals that might be exposed, or could otherwise be served better by having a repr that says something more than it's address/id.
A couple of previous tickets have addressed this in minor, one off ways - #23167, #22906, #22531 - but as I've begun collecting places they could supply useful info as I spot them in my stuff, they've become a hodgepodge across a number of modules.
For example, a couple I've got implemented:
<TemplateResponse status_code=200, charset=utf-8, _is_rendered=False> <LoaderOrigin name=/absolute/path/to/templates/admin/change_list.html, loadname=admin/change_list.html> <FilterExpression var=u'admin/change_list.html', filters=[]>
All of which, IMHO, provide a more useful context in which to evaluate state.
The branch I have implemented my reprs in is in no way merge ready yet, partially because there's no tests, and it'd need rebasing & squashing, but also because there are some parts I'd like to see reprs on but haven't landed on a good way to do so.
An example being the concept of a Loader
, which currently has no repr() and thus shows up as <path.to.cached.Loader … 0xETC>
which at least tells us which loader is being referenced. Given the convention so far for reprs() is just the class name (rather than module + class), the fact all Loaders (and now Engines) all share the same class name means in exposing more useful internal state we lose knowledge of the loader in question's module namespace.
As I write this, I've got at least rudimentary (as in, it was possibly to replace the 0x... with at least one object-state variable) reprs for:
added in e6406853c326ea9210d07766138d2b193c6d1fbaStaticNode
added in 66ed03e7c9ae4cd754aa918c6f4c9227cf424141BlockTranslateNode
added in d3ecef26b9fda02b88f925a800ae38dd5873c878URLNode
added in 179ee13eb37348cd87169a198aec18fedccc8668Token
added in 179ee13eb37348cd87169a198aec18fedccc8668Lexer
added in 179ee13eb37348cd87169a198aec18fedccc8668Parser
added in 179ee13eb37348cd87169a198aec18fedccc8668FilterExpression
added in dc86a25a677d05703e0bb021b178e44412cea7e9MiddlewareMixin
,RedirectFallbackMiddleware
,MessageMiddleware
,RemoteUserMiddleware
,CsrfViewMiddleware
added in 7171bf755b0c4be85ddbcc164eaf87164c131021Col
added in d3d95d645fc9f36355d27598475612de43587db1Lookup
added in 6fa2930573ff862a7f8ae8a11aa8bb7253fddeb7BaseDatabaseWrapper
added in 6fa2930573ff862a7f8ae8a11aa8bb7253fddeb7SQLCompiler
added in 6fa2930573ff862a7f8ae8a11aa8bb7253fddeb7JoinPromoter
added in c96f11257baf43188ff9daeddab3221992925c85HttpResponse
,HttpResponseRedirect
,HttpResponseNotAllowed
added in 9c40f01a66bd15457e10a0bbf28c803968b5dabbLazySettings
,Settings
, andUserSettingsHolder
added in d4b10a725614322769a419180039771634a06571GeoIP2
added in 22da686ca93e99f8091334f66d7199bc29b85c83PermWrapper
added in 7dd502b0e1e7d3e7fa2c5a2d54bbc926dbcf5f92BaseStorage
added in 7dd502b0e1e7d3e7fa2c5a2d54bbc926dbcf5f92ChangeList
added in 66ed03e7c9ae4cd754aa918c6f4c9227cf424141AdminForm
added in c96f11257baf43188ff9daeddab3221992925c85SimpleTemplateResponse
added in c96f11257baf43188ff9daeddab3221992925c85TemplateResponse
added in e4da3654367cd5f5c06ab109ece0042631faaa02AdminSite
added in e4da3654367cd5f5c06ab109ece0042631faaa02ModelAdmin
added in 4f4f770f77c5d720484c0a71f6a19b9affcc7e28Origin
added in 4f4f770f77c5d720484c0a71f6a19b9affcc7e28Template
added in 66ed03e7c9ae4cd754aa918c6f4c9227cf424141BlockContext
added in 66ed03e7c9ae4cd754aa918c6f4c9227cf424141IncludeNode
added in c609d5149c9295207cd7b2e8154e7b80a18d834aEngine
added in afb0eb8bb3e1c6f7ea842d2cede684eb4aec3f3dOrderedSet
added in bbb3965826c91406b94b819af7315ea1d39ec217StreamingHttpResponse
If accepted, this ticket could just track whatever I can end up getting reviewed/merged, or it could be a meta-ticket for accepting as a principle, and I could spin off tickets for more targeted areas.
Change History (57)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 10 years ago
comment:6 by , 8 years ago
Component: | Uncategorized → Core (Other) |
---|
comment:7 by , 4 years ago
Easy pickings: | set |
---|
Marking as easy pickings as I think this can be achieved in stages -- I'm not expecting someone to propose a PR that addresses all of these in one go!
comment:8 by , 4 years ago
In https://github.com/django/django/pull/14045: Refs #24121 -- Added repr to django.template.defaulttags.URLNode
comment:9 by , 4 years ago
Description: | modified (diff) |
---|
comment:11 by , 4 years ago
Description: | modified (diff) |
---|
comment:12 by , 4 years ago
Has patch: | set |
---|
comment:13 by , 4 years ago
Patch needs improvement: | set |
---|
comment:15 by , 4 years ago
Description: | modified (diff) |
---|---|
Has patch: | unset |
Patch needs improvement: | unset |
comment:18 by , 4 years ago
Description: | modified (diff) |
---|---|
Has patch: | unset |
comment:20 by , 4 years ago
Description: | modified (diff) |
---|
comment:21 by , 4 years ago
Description: | modified (diff) |
---|
comment:22 by , 4 years ago
Description: | modified (diff) |
---|
comment:23 by , 4 years ago
Query
Does not have __repr__
as well. Doesn't it need one?
Ah I just found it has __str__
comment:25 by , 4 years ago
Description: | modified (diff) |
---|
comment:27 by , 4 years ago
Description: | modified (diff) |
---|
comment:29 by , 4 years ago
Description: | modified (diff) |
---|
comment:30 by , 4 years ago
Description: | modified (diff) |
---|
comment:31 by , 4 years ago
Description: | modified (diff) |
---|
comment:34 by , 4 years ago
Description: | modified (diff) |
---|
comment:35 by , 4 years ago
Description: | modified (diff) |
---|
comment:36 by , 4 years ago
Description: | modified (diff) |
---|
comment:38 by , 4 years ago
Description: | modified (diff) |
---|
comment:40 by , 3 years ago
Description: | modified (diff) |
---|
comment:41 by , 3 years ago
Description: | modified (diff) |
---|
comment:44 by , 3 years ago
Description: | modified (diff) |
---|
comment:45 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:46 by , 3 years ago
add StreamingHttpResponse repr() method => https://github.com/django/django/pull/14531
comment:47 by , 3 years ago
Description: | modified (diff) |
---|
comment:48 by , 3 years ago
Description: | modified (diff) |
---|
comment:50 by , 3 years ago
Description: | modified (diff) |
---|
comment:52 by , 3 years ago
Description: | modified (diff) |
---|
comment:54 by , 3 years ago
Description: | modified (diff) |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
charset
,name
andloadname
should also be a string.Here's an example from Python: