[Django] #23626: Change coding style for sql, params return lines

Django

unread,

Oct 9, 2014, 10:29:00 AM10/9/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database | Keywords:
layer (models, ORM) | Has patch: 0
Severity: Normal | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
The current style can be quite confusing: {{{return '%s @> %s' % (lhs,
rhs), params}}}

This is much clearer:
{{{
sql = '{} @> {}'.format(lhs, rhs)
params = lhs_params + rhs_params
return sql, params
}}}

We should do this everywhere in the ORM or not at all.

(Originally brought up by Aymeric on
https://github.com/django/django/pull/3219)

--
Ticket URL: <https://code.djangoproject.com/ticket/23626>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Oct 9, 2014, 12:57:22 PM10/9/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

* easy: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:1>

Django

unread,

Oct 11, 2014, 5:28:22 PM10/11/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

Comment (by jpadilla):

This seems to be already taken care of, pretty sure it can be closed.

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:2>

Django

unread,

Oct 11, 2014, 5:48:55 PM10/11/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

Comment (by mjtamlyn):

This ticket was opened two days ago and there are no commits along those
lines I can see. For example I mean places like:
https://github.com/django/django/blob/bc46e4d4fa61eead13fe58048ae646f07d632e4f/django/db/models/lookups.py#L149-L154
https://github.com/django/django/blob/92a17eaae081a213171b044858d6fc29df2df733/django/contrib/postgres/fields/array.py#L167-L172

There aren't that many, and several are clearer to read than the examples
in that PR. The docs however suggest the current style used. It may be
worth looking more generally at string manipulation in the ORM and using
`.format()` instead of `%s` notation.

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:3>

Django

unread,

Oct 11, 2014, 9:04:07 PM10/11/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

Comment (by AmiZya):

@mjtamlyn should I create a new ticket for the formatting part in the ORM
?

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:4>

Django

unread,

Oct 12, 2014, 9:36:16 PM10/12/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

Changes (by AmiZya):

* cc: amizya@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:5>

Django

unread,

Oct 13, 2014, 10:16:53 AM10/13/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------
Reporter: mjtamlyn | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master

Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0

Keywords: afraid-to-commit | Patch needs improvement: 0

Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Changes (by EvilDMP):

* keywords: => afraid-to-commit

Comment:

I've marked this ticket as especially suitable for people following the
[https://dont-be-afraid-to-commit.readthedocs.org Don't be afraid to
commit tutorial] at the [http://python.ie/pycon/2014 PyCon Ireland 2014
sprints]. If you're tackling this ticket, please don't hesitate to ask me
for guidance if you'd like any, either here or on the Django IRC channels,
where I can be found as ''EvilDMP''.

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:6>

Django

unread,

Oct 14, 2014, 11:13:14 PM10/14/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------

* owner: nobody => pavel-shpilev
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:7>

Django

unread,

Nov 15, 2014, 10:47:19 AM11/15/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------

Reporter: mjtamlyn | Owner: aniav
Type: | Status: assigned
Cleanup/optimization | Version: master

Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by aniav):

* owner: pavel-shpilev => aniav

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:8>

Django

unread,

Nov 15, 2014, 1:24:00 PM11/15/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------

Reporter: mjtamlyn | Owner: aniav
Type: | Status: assigned
Cleanup/optimization | Version: master

Comment (by aniav):

There are quite a lot places where `%s` is used in ORM string manipulation
and I am not quite sure that changing to format is a good idea. It's
actually not helping much with readability especially in base SQL strings
defined in `db/backend/schema.py`. Curly braces are also used in T-SQL for
escaping Date, Time and Timestamp, and in PostgreSQL for supplying Array
values - this wouldn't make the issue an easy picking one IMO.
I am not really sure if changing that would have any positive outcome. I
would just abandon the idea.

--
Ticket URL: <https://code.djangoproject.com/ticket/23626#comment:9>

Django

unread,

Nov 16, 2014, 4:50:16 AM11/16/14

to django-...@googlegroups.com

#23626: Change coding style for sql, params return lines
-------------------------------------+-------------------------------------

Reporter: mjtamlyn | Owner: aniav
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Database layer | Resolution: wontfix