Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#28769 closed Cleanup/optimization (fixed)

Utilize 'x or y' in place of 'x if x else y'

Reported by: Дилян Палаузов Owned by: nobody
Component: Core (Other) Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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])

Change History (5)

comment:1 by Tim Graham, 6 years ago

Component: UncategorizedCore (Other)
Type: UncategorizedCleanup/optimization

I'm not sure if that's an improvement. 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".

comment:2 by Дилян Палаузов, 6 years ago

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))

comment:3 by Tim Graham, 6 years ago

Has patch: set
Triage Stage: UnreviewedReady for checkin

comment:4 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: newclosed

In c69e4bc:

Fixed #28769 -- Replaced 'x if x else y' with 'x or y'.

comment:5 by Adam Johnson, 6 years ago

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
Note: See TracTickets for help on using tickets.
Back to Top