Code

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#244 closed enhancement (wontfix)

[patch] Make new get_values() function more forgiving of mistakes

Reported by: rmunn@… Owned by: adrian
Component: Metasystem Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Thanks for applying my patch from #214! There's one part of it that got omitted, though, and I'm wondering why: it's the part that Does The Right Thing if the user passes a string for the fields=... kwarg. When testing the patch, I found that when I only wanted one field, it was a natural mistake to forget to pass it as a list. I was doing things like "get_values(fields='name', distinct=True" instead of "get_values(fields=['name'], distinct=True". And this was my own code I was calling: if I was making that mistake, others will be also. In the same spirit as [213], I think we should be more forgiving of this mistake: what the user meant is unambiguous, so there's no harm in allowing it.

Here's a patch to implement this idea:

Index: django/core/meta.py
===================================================================
--- django/core/meta.py (revision 359)
+++ django/core/meta.py (working copy)
@@ -1100,6 +1100,10 @@
     except KeyError: # Default to all fields.
         fields = [f.name for f in opts.fields]

+    if isinstance(fields, basestring):
+        # Be forgiving of mistakes: convert to list
+        fields = [fields]
+
     cursor = db.db.cursor()
     _, sql, params = function_get_sql_clause(opts, **kwargs)
     select = ['%s.%s' % (opts.db_table, f) for f in fields]

Attachments (0)

Change History (2)

comment:1 Changed 9 years ago by adrian

  • Resolution set to wontfix
  • Status changed from new to closed

I talked it over with Jacob, and we agreed strongly that this is too much DWIM for our tastes.

comment:2 Changed 9 years ago by mmarshall

So, perhaps it still should check if a string was given, and give a friendly error if so?

MWM

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.