[Django] #28769: Utilize 'x or y' in place of 'x if x else y'

15 views
Skip to first unread message

Django

unread,
Nov 4, 2017, 5:56:22 PM11/4/17
to django-...@googlegroups.com
#28769: Utilize 'x or y' in place of 'x if x else y'
------------------------------------------+------------------------
Reporter: Дилян Палаузов | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
{{{
diff --git a/django/contrib/admin/bin/compress.py
b/django/contrib/admin/bin/compress.py
--- a/django/contrib/admin/bin/compress.py
+++ b/django/contrib/admin/bin/compress.py
@@ -28,7 +28,7 @@ Compiler library and Java version 6 or later."""
parser.add_argument("-q", "--quiet", action="store_false",
dest="verbose")
options = parser.parse_args()

- compiler = Path(closure_compiler if closure_compiler else
options.compiler).expanduser()
+ compiler = Path(closure_compiler or options.compiler).expanduser()
if not compiler.exists():
sys.exit(
"Google Closure compiler jar file %s not found. Please use
the -c "
diff --git a/django/contrib/gis/gdal/raster/base.py
b/django/contrib/gis/gdal/raster/base.py
--- a/django/contrib/gis/gdal/raster/base.py
+++ b/django/contrib/gis/gdal/raster/base.py
@@ -56,7 +56,7 @@ class GDALRasterBase(GDALBase):
counter += 1
item = data[counter]
# The default domain values are returned if domain is None.
- result[domain if domain else 'DEFAULT'] = domain_meta
+ result[domain or 'DEFAULT'] = domain_meta
return result

@metadata.setter
diff --git a/django/core/management/commands/makemessages.py
b/django/core/management/commands/makemessages.py
--- a/django/core/management/commands/makemessages.py
+++ b/django/core/management/commands/makemessages.py
@@ -323,9 +323,9 @@ class Command(BaseCommand):
raise CommandError("currently makemessages only supports
domains "
"'django' and 'djangojs'")
if self.domain == 'djangojs':
- exts = extensions if extensions else ['js']
+ exts = extensions or ['js']
else:
- exts = extensions if extensions else ['html', 'txt', 'py']
+ exts = extensions or ['html', 'txt', 'py']
self.extensions = handle_extensions(exts)

if (locale is None and not exclude and not process_all) or
self.domain is None:
diff --git a/django/db/backends/base/introspection.py
b/django/db/backends/base/introspection.py
--- a/django/db/backends/base/introspection.py
+++ b/django/db/backends/base/introspection.py
@@ -130,7 +130,7 @@ class BaseDatabaseIntrospection:
# we don't need to reset the sequence.
if f.remote_field.through is None:
sequence = self.get_sequences(cursor,
f.m2m_db_table())
- sequence_list.extend(sequence if sequence else
[{'table': f.m2m_db_table(), 'column': None}])
+ sequence_list.extend(sequence or [{'table':
f.m2m_db_table(), 'column': None}])
return sequence_list

def get_sequences(self, cursor, table_name, table_fields=()):
diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
--- a/django/db/models/sql/query.py
+++ b/django/db/models/sql/query.py
@@ -606,7 +606,7 @@ class Query:

# Ordering uses the 'rhs' ordering, unless it has none, in which
case
# the current ordering is used.
- self.order_by = rhs.order_by if rhs.order_by else self.order_by
+ self.order_by = rhs.order_by or self.order_by
self.extra_order_by = rhs.extra_order_by or self.extra_order_by

def deferred_to_data(self, target, callback):
diff --git a/django/forms/widgets.py b/django/forms/widgets.py
--- a/django/forms/widgets.py
+++ b/django/forms/widgets.py
@@ -474,7 +474,7 @@ class DateTimeBaseInput(TextInput):

def __init__(self, attrs=None, format=None):
super().__init__(attrs)
- self.format = format if format else None
+ self.format = format or None

def format_value(self, value):
return formats.localize_input(value, self.format or
formats.get_format(self.format_key)[0])
}}}

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

Django

unread,
Nov 4, 2017, 6:26:34 PM11/4/17
to django-...@googlegroups.com
#28769: Utilize 'x or y' in place of 'x if x else y'
-------------------------------------+-------------------------------------

Reporter: Дилян Палаузов | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* component: Uncategorized => Core (Other)
* type: Uncategorized => Cleanup/optimization


Comment:

I'm not sure if that's an improvement.
[https://www.python.org/dev/peps/pep-0308/ Conditional expressions] were
added to Python to address "the prevalence of error-prone attempts to
achieve the same effect using 'and' and 'or'". I think they're more
explicit than using "or".

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

Django

unread,
Nov 4, 2017, 6:38:31 PM11/4/17
to django-...@googlegroups.com
#28769: Utilize 'x or y' in place of 'x if x else y'
-------------------------------------+-------------------------------------

Reporter: Дилян Палаузов | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Дилян Палаузов):

This is already inconsistent in the code:
{{{
django/db/migrations/operations/models.py:48: self.options =
options or {}
django/db/migrations/operations/models.py:49: self.bases = bases or
(models.Model,)
django/db/migrations/operations/special.py:17:
self.database_operations = database_operations or []
django/db/migrations/operations/special.py:18:
self.state_operations = state_operations or []
django/db/migrations/operations/special.py:75:
self.state_operations = state_operations or []
django/db/migrations/operations/special.py:76: self.hints = hints
or {}
django/db/migrations/operations/special.py:153: self.hints = hints
or {}
django/db/migrations/state.py:88: self.models = models or {}
django/db/migrations/state.py:90: self.real_apps = real_apps or []
django/db/migrations/state.py:363: self.options = options or {}
django/db/migrations/state.py:365: self.bases = bases or
(models.Model, )
django/db/migrations/state.py:366: self.managers = managers or []
}}}

Adding ternary expression to avoid using incorrectly or/and is fine, but
it was not added to start using "x if x else y" instead of "x or y".

There are more places which can be shortened this way:
{{{
diff --git a/django/contrib/sessions/backends/file.py
b/django/contrib/sessions/backends/file.py
--- a/django/contrib/sessions/backends/file.py
+++ b/django/contrib/sessions/backends/file.py
@@ -73,10 +73,7 @@ class SessionStore(SessionBase):
"""
Return the expiry time of the file storing the session's content.
"""
- expiry = session_data.get('_session_expiry')
- if not expiry:
- expiry = self._last_modification() +
datetime.timedelta(seconds=settings.SESSION_COOKIE_AGE)
- return expiry
+ return session_data.get('_session_expiry') or
self._last_modification() +
datetime.timedelta(seconds=settings.SESSION_COOKIE_AGE)

def load(self):
session_data = {}
diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py
--- a/django/core/handlers/wsgi.py
+++ b/django/core/handlers/wsgi.py
@@ -66,13 +66,11 @@ class LimitedStream:
class WSGIRequest(HttpRequest):
def __init__(self, environ):
script_name = get_script_name(environ)
- path_info = get_path_info(environ)
- if not path_info:
+ path_info = get_path_info(environ) or '/'
# Sometimes PATH_INFO exists, but is empty (e.g. accessing
# the SCRIPT_NAME URL without a trailing slash). We really
need to
# operate as if they'd requested '/'. Not amazingly nice to
force
# the path like this, but should be harmless.
- path_info = '/'
self.environ = environ
self.path_info = path_info
# be careful to only replace the first slash in the path because
of


diff --git a/django/core/management/commands/makemessages.py
b/django/core/management/commands/makemessages.py
--- a/django/core/management/commands/makemessages.py
+++ b/django/core/management/commands/makemessages.py

@@ -501,9 +501,7 @@ class Command(BaseCommand):
locale_dir = path
break
if not locale_dir:
- locale_dir = self.default_locale_path
- if not locale_dir:
- locale_dir = NO_LOCALE_DIR
+ locale_dir = self.default_locale_path or
NO_LOCALE_DIR
all_files.append(self.translatable_file_class(dirpath, filename,
locale_dir))
return sorted(all_files)

diff --git a/django/db/backends/sqlite3/creation.py
b/django/db/backends/sqlite3/creation.py
--- a/django/db/backends/sqlite3/creation.py
+++ b/django/db/backends/sqlite3/creation.py
@@ -12,9 +12,7 @@ class DatabaseCreation(BaseDatabaseCreation):
return database_name == ':memory:' or 'mode=memory' in
database_name

def _get_test_db_name(self):
- test_database_name =
self.connection.settings_dict['TEST']['NAME']
- if not test_database_name:
- test_database_name = ':memory:'
+ test_database_name =
self.connection.settings_dict['TEST']['NAME'] or ':memory:'
if test_database_name == ':memory:':
return 'file:memorydb_%s?mode=memory&cache=shared' %
self.connection.alias
return test_database_name
diff --git a/django/utils/dateparse.py b/django/utils/dateparse.py
--- a/django/utils/dateparse.py
+++ b/django/utils/dateparse.py
@@ -131,9 +131,7 @@ def parse_duration(value):
Also supports ISO 8601 representation and PostgreSQL's day-time
interval
format.
"""
- match = standard_duration_re.match(value)
- if not match:
- match = iso8601_duration_re.match(value) or
postgres_interval_re.match(value)
+ match = standard_duration_re.match(value) or
iso8601_duration_re.match(value) or postgres_interval_re.match(value)
if match:
kw = match.groupdict()
days = datetime.timedelta(float(kw.pop('days', 0) or 0))
}}}

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

Django

unread,
Nov 6, 2017, 10:28:07 AM11/6/17
to django-...@googlegroups.com
#28769: Utilize 'x or y' in place of 'x if x else y'
-------------------------------------+-------------------------------------

Reporter: Дилян Палаузов | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* has_patch: 0 => 1
* stage: Unreviewed => Ready for checkin


Comment:

[https://github.com/django/django/pull/9328 PR]

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

Django

unread,
Nov 7, 2017, 9:25:35 AM11/7/17
to django-...@googlegroups.com
#28769: Utilize 'x or y' in place of 'x if x else y'
-------------------------------------+-------------------------------------

Reporter: Дилян Палаузов | Owner: nobody
Type: | Status: closed

Cleanup/optimization |
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"c69e4bc69166b2d752b437a651dfa91f8b53ecfd" c69e4bc]:
{{{
#!CommitTicketReference repository=""
revision="c69e4bc69166b2d752b437a651dfa91f8b53ecfd"
Fixed #28769 -- Replaced 'x if x else y' with 'x or y'.
}}}

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

Django

unread,
Nov 12, 2017, 6:01:33 PM11/12/17
to django-...@googlegroups.com
#28769: Utilize 'x or y' in place of 'x if x else y'
-------------------------------------+-------------------------------------

Reporter: Дилян Палаузов | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Core (Other) | Version: 1.11
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam (Chainz) Johnson):

Nice, this is also a little faster (at least, fewer opcodes, the time
difference is minimal):

{{{
> default ~ 👾 ❯ ipython
Python 3.6.0 (default, Dec 24 2016, 08:01:42)
Type "copyright", "credits" or "license" for more information.

IPython 5.3.0 -- An enhanced Interactive Python.
? -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help -> Python's own help system.
object? -> Details about 'object', use 'object??' for extra details.

In [1]:
Do you really want to exit ([y]/n)? n

In [1]: def foo(a=1, b=2):
...: return a if a else b
...:

In [2]: def bar(a=1, b=2):
...: return a or b
...:

In [3]: import dis

In [4]: dis.dis(foo)
2 0 LOAD_FAST 0 (a)
2 POP_JUMP_IF_FALSE 8
4 LOAD_FAST 0 (a)
6 RETURN_VALUE
>> 8 LOAD_FAST 1 (b)
10 RETURN_VALUE

In [5]: dis.dis(bar)
2 0 LOAD_FAST 0 (a)
2 JUMP_IF_TRUE_OR_POP 6
4 LOAD_FAST 1 (b)
>> 6 RETURN_VALUE

In [6]: %timeit foo()
The slowest run took 8.95 times longer than the fastest. This could mean
that an intermediate result is being cached.
10000000 loops, best of 3: 115 ns per loop

In [7]: %timeit bar()
10000000 loops, best of 3: 108 ns per loop
}}}

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

Reply all
Reply to author
Forward
0 new messages