Code

Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#163 closed enhancement (fixed)

Option to leave off __exact

Reported by: Manuzhai Owned by: adrian
Component: Template system Version: magic-removal
Severity: major Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by adrian)

Doing sites.get_object(id__exact = 1) seems kind of verbose if sites.get_object(id = 1) would convey the same meaning and be a little easier to read.

Attachments (0)

Change History (13)

comment:1 Changed 9 years ago by Manuzhai

Okay, so I meant

id__exact

comment:2 Changed 9 years ago by adrian

  • Description modified (diff)

comment:3 Changed 9 years ago by adrian

My thoughts on this: Explicit is better than implicit.

comment:4 Changed 9 years ago by rmunn@…

But not if what's implicit is blindingly obvious.

The option of idexact should be kept, but I can't conceive of any other meaning for polls.get_object(id=5) than id__exact=5. Beautiful is better than ugly. Simple is better than complex. Readability counts. I'll see your Zen and raise you two more. :-)

comment:5 Changed 9 years ago by rmunn@…

Argh, should have put triple-braces around that id__exact. What I meant to say was:

The option of id__exact should be kept, but I can't conceive of any other meaning for polls.get_object(id=5) than id__exact=5.

Beautiful is better than ugly.
Simple is better than complex.
Readability counts.

I'll see your Zen quote and raise you two more. :-)

comment:6 Changed 9 years ago by rmunn@…

Besides, look at how short the patch is!

Index: django/core/meta.py
===================================================================
--- django/core/meta.py	(revision 299)
+++ django/core/meta.py	(working copy)
@@ -1142,8 +1147,10 @@
             continue
         lookup_list = kwarg.split(LOOKUP_SEPARATOR)
         if len(lookup_list) == 1:
-            _throw_bad_kwarg_error(kwarg)
-        lookup_type = lookup_list.pop()
+            # polls.get_object(id=5) is shorthand for polls.get_object(id__exact=5)
+            lookup_type = 'exact'
+        else:
+            lookup_type = lookup_list.pop()
         current_opts = opts # We'll be overwriting this, so keep a reference to the original opts.
         current_table_alias = current_opts.db_table
         param_required = False

Isn't it the cutest patch you've ever seen? It followed me home, Mom, can I please apply it to the repository? :-)

On a serious note: this will only work for lookups that are one "level" of complexity deep, e.g. get_object(id=5). I thought about the possibility of doing choices.get_list(polls__sites__id=5) instead of choices.get_list(polls__sites__id__exact=5) and realized no, that wouldn't be a good idea. If you're writing out a relation of this much complexity, it's better to be explicit about the fact that you're doing an exact match. But in the vast majority of cases where you'd want to use the shorthand, you're doing something like get_object(id=5).

comment:7 Changed 9 years ago by jacob

See the discussion on this ticket here: http://loglibrary.com/show_page/view/179?Multiplier=3600&Interval=6&StartTime=1122065878 and the resulting proposal here: http://groups-beta.google.com/group/django-developers/browse_thread/thread/6eb17e5d996ed393/ab1cee3ad60980a7#ab1cee3ad60980a7

Basically: add a magic "pk=" keyword to lookups that would do a "primarykeyfieldexact=whatever" lookup for you.

comment:8 Changed 9 years ago by rmunn@…

Here's a patch to use the magic "pk=(value)" syntax.

Index: django/core/meta.py
===================================================================
--- django/core/meta.py (revision 300)
+++ django/core/meta.py (working copy)
@@ -1142,7 +1147,12 @@
             continue
         lookup_list = kwarg.split(LOOKUP_SEPARATOR)
         if len(lookup_list) == 1:I tested against SQLite;
-            _throw_bad_kwarg_error(kwarg)
+            if kwarg == 'pk':
+                # "pk=value" is shorthand for "(primary key)__exact=value"
+                lookup_list = [opts.pk.name, 'exact']
+            else:
+                # Not shorthand, just a bad kwarg
+                _throw_bad_kwarg_error(kwarg)
         lookup_type = lookup_list.pop()
         current_opts = opts # We'll be overwriting this, so keep a reference to the original opts.
         current_table_alias = current_opts.db_table

Again, it would be possible to do this at deeper levels, but I think the risk of ambiguity isn't worth the effort saved. Besides, if you're going through one table to get to the next table, you probably don't have an id into the second table anyway.

comment:9 Changed 9 years ago by Clint Ecker <clintecker@…>

I would like it if sites.get_object(1) would default to using the primary key. I personally think id= is a little verbose myself :)

comment:10 Changed 9 years ago by adrian

  • Status changed from new to assigned

comment:11 Changed 9 years ago by adrian

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [316]) Fixed #163 -- Added 'pk' database API option, which is a shorthand for (primary_key)exact

comment:12 Changed 8 years ago by anonymous

  • Component changed from Admin interface to Template system
  • milestone set to Version 1.0
  • priority changed from normal to low
  • Severity changed from normal to major
  • Type changed from defect to enhancement
  • Version set to magic-removal

comment:13 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.