Opened 6 years ago

Closed 6 years ago

#28906 closed Cleanup/optimization (fixed)

Reduce calls to bool()

Reported by: Дилян Палаузов Owned by: nobody
Component: Core (Other) Version: dev
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

Compared to the attached patch, these changes might be more controversial:

diff --git a/django/contrib/gis/db/backends/spatialite/features.py b/django/contrib/gis/db/backends/spatialite/features
--- a/django/contrib/gis/db/backends/spatialite/features.py
+++ b/django/contrib/gis/db/backends/spatialite/features.py
@@ -10,4 +10,4 @@ class DatabaseFeatures(BaseSpatialFeatures, SQLiteDatabaseFeatures):
 
     @cached_property
     def supports_area_geodetic(self):
-        return bool(self.connection.ops.lwgeom_version())
+        return self.connection.ops.lwgeom_version()
diff --git a/django/db/models/sql/compiler.py b/django/db/models/sql/compiler.py
--- a/django/db/models/sql/compiler.py
+++ b/django/db/models/sql/compiler.py
@@ -51,7 +51,7 @@ class SQLCompiler:
         order_by = self.get_order_by()
         self.where, self.having = self.query.where.split_having()
         extra_select = self.get_extra_select(order_by, self.select)
-        self.has_extra_select = bool(extra_select)
+        self.has_extra_select = extra_select
         group_by = self.get_group_by(self.select + extra_select, order_by)
         return extra_select, order_by, group_by
 @@ -1216,11 +1216,10 @@ class SQLInsertCompiler(SQLCompiler):
         insert_statement = self.connection.ops.insert_statement(on_conflict=self.query.on_conflict)
         result = ['%s %s' % (insert_statement, qn(opts.db_table))]
 
-        has_fields = bool(self.query.fields)
-        fields = self.query.fields if has_fields else [opts.pk]
+        fields = self.query.fields or [opts.pk]
         result.append('(%s)' % ', '.join(qn(f.column) for f in fields))
 
-        if has_fields:
+        if self.query.fields:
             value_rows = [
                 [self.prepare_value(field, self.pre_save_val(field, obj)) for field in fields]
                 for obj in self.query.objs

Attachments (1)

less_bool.patch (5.3 KB ) - added by Дилян Палаузов 6 years ago.

Download all attachments as: .zip

Change History (4)

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

Attachment: less_bool.patch added

comment:1 by Jon Dufresne, 6 years ago

     @cached_property
     def supports_area_geodetic(self):
-        return bool(self.connection.ops.lwgeom_version())
+        return self.connection.ops.lwgeom_version()
-        self.has_extra_select = bool(extra_select)
+        self.has_extra_select = extra_select

For methods and members, I think the type might be considered part of the interface and should perhaps stay.

Reducing bool() in expressions seems OK to me though.

comment:2 by Tim Graham, 6 years ago

Component: UncategorizedCore (Other)
Triage Stage: UnreviewedReady for checkin

PR. I updated the patch as per Jon's comments which I also agree with.

comment:3 by GitHub <noreply@…>, 6 years ago

Resolution: fixed
Status: newclosed

In 2b81faa:

Fixed #28906 -- Removed unnecessary bool() calls.

Note: See TracTickets for help on using tickets.
Back to Top