#244 closed enhancement (wontfix)
[patch] Make new get_values() function more forgiving of mistakes
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
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: | no | UI/UX: | no |
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]
Change History (2)
comment:1 by , 19 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 19 years ago
So, perhaps it still should check if a string was given, and give a friendly error if so?
MWM
I talked it over with Jacob, and we agreed strongly that this is too much DWIM for our tastes.