Sourcery suggested refactorings #152
Conversation
| if xlwt is not None: | ||
| column.xlwt_stymat = self.xlwt_stymat_init() | ||
| else: | ||
| column.xlwt_stymat = None | ||
| column.xlwt_stymat = self.xlwt_stymat_init() if xlwt is not None else None |
There was a problem hiding this comment.
Function Column.new_instance refactored with the following changes:
- Replace if statement with if expression (assign-if-exp)
| if arrow and isinstance(data, arrow.Arrow): | ||
| if data.strftime(format) == format: | ||
| return data.format(format) | ||
| if ( | ||
| arrow | ||
| and isinstance(data, arrow.Arrow) | ||
| and data.strftime(format) == format | ||
| ): | ||
| return data.format(format) |
There was a problem hiding this comment.
Function DateColumnBase._format_datetime refactored with the following changes:
- Merge nested if conditions (merge-nested-ifs)
| else: | ||
| sort_display.append(col.key) | ||
| redundant.append(col.key) | ||
| sort_display.append(col.key) | ||
| redundant.append(col.key) |
There was a problem hiding this comment.
Function BaseGrid.query_sort refactored with the following changes:
- Remove unnecessary else after guard condition (remove-unnecessary-else)
| if isinstance(obj, datetime.date) or isinstance(obj, arrow.Arrow): | ||
| if isinstance(obj, (datetime.date, arrow.Arrow)): |
There was a problem hiding this comment.
Function CustomJsonEncoder.default refactored with the following changes:
- Merge isinstance calls (merge-is-instance)
| if data: | ||
| current_args = self.json_to_args(data) | ||
| else: | ||
| current_args = MultiDict() | ||
| current_args = self.json_to_args(data) if data else MultiDict() |
There was a problem hiding this comment.
Function RequestJsonLoader.get_args refactored with the following changes:
- Replace if statement with if expression (assign-if-exp)
| web_session['dgsessions'] = dict() | ||
| web_session['dgsessions'] = {} |
There was a problem hiding this comment.
Function WebSessionArgsLoader.save_session_store refactored with the following changes:
- Replace dict() with {} (dict-literal)
| if grid.session_on: | ||
| self.remove_grid_session(previous_args.get('session_key') or grid.session_key) | ||
| self.remove_grid_session(grid.default_session_key) | ||
| self.remove_grid_session(previous_args.get('session_key') or grid.session_key) | ||
| self.remove_grid_session(grid.default_session_key) |
There was a problem hiding this comment.
Function WebSessionArgsLoader.get_args refactored with the following changes:
- Remove redundant conditional (remove-redundant-if)
| else: | ||
| # if its not the string 'auto' and its not a formencode validator, assume | ||
| # its a callable and wrap with a formencode validator | ||
| if not hasattr(self.value_modifier, 'to_python'): | ||
| if not hasattr(self.value_modifier, '__call__'): | ||
| raise TypeError( | ||
| _('value_modifier must be the string "auto", have a "to_python" attribute, ' | ||
| 'or be a callable') | ||
| ) | ||
| self.value_modifier = feval.Wrapper(to_python=self.value_modifier) | ||
| elif not hasattr(self.value_modifier, 'to_python'): | ||
| if not hasattr(self.value_modifier, '__call__'): | ||
| raise TypeError( | ||
| _('value_modifier must be the string "auto", have a "to_python" attribute, ' | ||
| 'or be a callable') | ||
| ) | ||
| self.value_modifier = feval.Wrapper(to_python=self.value_modifier) |
There was a problem hiding this comment.
Function OptionsFilterBase.setup_validator refactored with the following changes:
- Merge else clause's nested if statement into elif (merge-else-if-into-elif)
| if self.op in (ops.is_, ops.not_is) and not (self.value1 or self.default_op): | ||
| self.op = None | ||
| if ( | ||
| self.op in (ops.is_, ops.not_is) | ||
| and not self.value1 | ||
| and not self.default_op | ||
| ): self.op = None |
There was a problem hiding this comment.
Function OptionsFilterBase.set refactored with the following changes:
- Simplify logical expression using De Morgan identities (de-morgan)
| target_date = first_day if first_day else last_day | ||
| target_date = first_day or last_day |
There was a problem hiding this comment.
Function _DateMixin.description refactored with the following changes:
- Simplify if expression by using or (or-if-exp-identity)
| if is_value2: | ||
| if self.op in (ops.between, ops.not_between): | ||
| value = '' | ||
| else: | ||
| return None | ||
| else: | ||
| if not is_value2: | ||
| raise formencode.Invalid(gettext('invalid date'), value, self) | ||
|
|
||
| if self.op in (ops.between, ops.not_between): | ||
| value = '' | ||
| else: | ||
| return None |
There was a problem hiding this comment.
Function DateFilter.process refactored with the following changes:
- Swap if/else branches (swap-if-else-branches)
- Remove unnecessary else after guard condition (remove-unnecessary-else)
| if is_value2: | ||
| if self.op in (ops.between, ops.not_between): | ||
| value = '' | ||
| else: | ||
| return None | ||
| else: | ||
| if not is_value2: | ||
| raise formencode.Invalid(gettext('invalid date'), value, self) | ||
|
|
||
| if self.op in (ops.between, ops.not_between): | ||
| value = '' | ||
| else: | ||
| return None |
There was a problem hiding this comment.
Function DateTimeFilter.process refactored with the following changes:
- Swap if/else branches (swap-if-else-branches)
- Remove unnecessary else after guard condition (remove-unnecessary-else)
| for col in self.columns: | ||
| if col.group: | ||
| return True | ||
|
|
||
| return False | ||
| return any(col.group for col in self.columns) |
There was a problem hiding this comment.
Function GroupMixin.has_groups refactored with the following changes:
- Use any() instead of for loop (use-any)
| rows = [] | ||
| for col in six.itervalues(self.grid.filtered_cols): | ||
| rows.append(self.filtering_table_row(col)) | ||
| rows = [ | ||
| self.filtering_table_row(col) | ||
| for col in six.itervalues(self.grid.filtered_cols) | ||
| ] |
There was a problem hiding this comment.
Function HTML.filtering_fields refactored with the following changes:
- Convert for loop into list comprehension (list-comprehension)
| if not filter.is_display_active: | ||
| current_selected = '' | ||
| else: | ||
| current_selected = filter.op | ||
| current_selected = '' if not filter.is_display_active else filter.op |
There was a problem hiding this comment.
Function `HTML.filtering_col_op_selectz refactored with the following changes:
- Replace if statement with if expression (assign-if-exp)
| headings = [] | ||
| for col in self.columns: | ||
| headings.append(self.table_th(col)) | ||
| headings = [self.table_th(col) for col in self.columns] |
There was a problem hiding this comment.
Function HTML.table_column_headings refactored with the following changes:
- Convert for loop into list comprehension (list-comprehension)
| url_args = {} | ||
| url_args['dgreset'] = None | ||
| url_args['sort2'] = None | ||
| url_args['sort3'] = None | ||
| url_args = {'dgreset': None, 'sort2': None, 'sort3': None} |
There was a problem hiding this comment.
Function HTML.table_th refactored with the following changes:
- Merge dictionary assignment with declaration (merge-dict-assign)
| if (rownum + 1) % 2 == 1: | ||
| row_hah.class_ += 'odd' | ||
| else: | ||
| row_hah.class_ += 'even' | ||
| row_hah.class_ += 'odd' if (rownum + 1) % 2 == 1 else 'even' |
There was a problem hiding this comment.
Function HTML.table_tr_styler refactored with the following changes:
- Replace if statement with if expression (assign-if-exp)
| cells = [] | ||
| for col in self.columns: | ||
| cells.append(self.table_td(col, record)) | ||
| cells = [self.table_td(col, record) for col in self.columns] |
There was a problem hiding this comment.
Function HTML.table_tr refactored with the following changes:
- Convert for loop into list comprehension (list-comprehension)
| buffer_hah = { | ||
| 'colspan': colspan, | ||
| 'class': 'totals-label' | ||
| } | ||
| if colspan: | ||
| buffer_hah = { | ||
| 'colspan': colspan, | ||
| 'class': 'totals-label' | ||
| } |
There was a problem hiding this comment.
Function HTML.table_totals refactored with the following changes:
- Move assignments closer to their usage (move-assign)
| url_args = {} | ||
| url_args['perpage'] = None | ||
| url_args['onpage'] = None | ||
| url_args['search'] = None | ||
| url_args['sort1'] = None | ||
| url_args['sort2'] = None | ||
| url_args['sort3'] = None | ||
| url_args['export_to'] = None | ||
| url_args['datagrid-add-filter'] = None | ||
| url_args = { | ||
| 'perpage': None, | ||
| 'onpage': None, | ||
| 'search': None, | ||
| 'sort1': None, | ||
| 'sort2': None, | ||
| 'sort3': None, | ||
| 'export_to': None, | ||
| 'datagrid-add-filter': None, | ||
| } |
There was a problem hiding this comment.
Function HTML.reset_url refactored with the following changes:
- Merge dictionary assignment with declaration (merge-dict-assign)
| headings = [] | ||
| for col in self.columns: | ||
| headings.append(col.label) | ||
| headings = [col.label for col in self.columns] |
There was a problem hiding this comment.
Function CSV.body_headings refactored with the following changes:
- Convert for loop into list comprehension (list-comprehension)
| row = [] | ||
| for col in self.columns: | ||
| row.append(col.render('csv', record)) | ||
| row = [col.render('csv', record) for col in self.columns] |
There was a problem hiding this comment.
Function CSV.body_records refactored with the following changes:
- Convert for loop into list comprehension (list-comprehension)
Sourcery refactored this code to make it cleaner and more readable.
If you want Sourcery to review the full project or all new pull requests, add Sourcery to your repo.
We try to only open PRs that are helpful. If this isn't helpful or you have any feedback for us - please let us know!