-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sourcery suggested refactorings #152
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -289,10 +289,7 @@ def new_instance(self, grid): | |
column.head.hah = HTMLAttributes(self.kwargs) | ||
column.body = BlankObject() | ||
column.body.hah = HTMLAttributes(self.kwargs) | ||
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 | ||
|
||
# try to be smart about which attributes should get copied to the | ||
# new instance by looking for attributes on the class that have the | ||
|
@@ -554,9 +551,12 @@ def __init__(self, label, key_or_filter=None, key=None, can_sort=True, | |
|
||
def _format_datetime(self, data, format): | ||
# if we have an arrow date, allow html_format to use that functionality | ||
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) | ||
Comment on lines
-557
to
+559
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
return data.strftime(format) | ||
|
||
def render_html(self, record, hah): | ||
|
@@ -1586,9 +1586,8 @@ def query_sort(self, query): | |
# remove any redundant names, whichever comes first is what we will keep | ||
if col.key in redundant: | ||
continue | ||
else: | ||
sort_display.append(col.key) | ||
redundant.append(col.key) | ||
sort_display.append(col.key) | ||
redundant.append(col.key) | ||
Comment on lines
-1589
to
+1590
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
query = col.apply_sort(query, flag_desc) | ||
if sort_display: | ||
log.debug(','.join(sort_display)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ def ngettext(singular, plural, num, **variables): | |
|
||
class CustomJsonEncoder(json.JSONEncoder): | ||
def default(self, obj): | ||
if isinstance(obj, datetime.date) or isinstance(obj, arrow.Arrow): | ||
if isinstance(obj, (datetime.date, arrow.Arrow)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
return obj.isoformat() | ||
|
||
try: | ||
|
@@ -154,10 +154,7 @@ def json_to_args(self, data: Dict[str, Any]): | |
|
||
def get_args(self, grid, previous_args): | ||
data = self.manager.request_json() | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
current_args.update(previous_args) | ||
return current_args | ||
|
||
|
@@ -341,7 +338,7 @@ def save_session_store(self, grid, args): | |
# and also as the default args for this grid | ||
web_session = self.manager.web_session() | ||
if 'dgsessions' not in web_session: | ||
web_session['dgsessions'] = dict() | ||
web_session['dgsessions'] = {} | ||
Comment on lines
-344
to
+341
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
dgsessions = web_session['dgsessions'] | ||
grid_session_key = args.get('session_key') or grid.session_key | ||
# work with a copy here | ||
|
@@ -406,9 +403,8 @@ def get_args(self, grid, previous_args): | |
|
||
if 'dgreset' in previous_args: | ||
# Request has a reset. Do nothing else. | ||
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) | ||
Comment on lines
-409
to
+407
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
return MultiDict(dict(dgreset=1, session_key=previous_args.get('session_key'))) | ||
|
||
# From here on, work with a copy so as not to mutate the incoming args | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -455,16 +455,13 @@ def setup_validator(self): | |
_("can't use value_modifier='auto' when option keys are {key_type}", | ||
key_type=type(first_key)) | ||
) | ||
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) | ||
Comment on lines
-458
to
+464
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
def set(self, op, values, value2=None): | ||
"""Set the filter op/values to be used by query modifiers. | ||
|
@@ -507,8 +504,11 @@ def set(self, op, values, value2=None): | |
# | ||
# however, we have to test the operator first, because if it is empty | ||
# or !empty, then it would make sense for self.value1 to be empty. | ||
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 | ||
Comment on lines
-510
to
+511
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
self.value1_set_with = values | ||
|
||
|
@@ -971,7 +971,7 @@ def description(self): | |
second_date=last_day.strftime('%m/%d/%Y')) | ||
else: | ||
# !!!: localize | ||
target_date = first_day if first_day else last_day | ||
target_date = first_day or last_day | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
return _('{descriptor}{date}', | ||
descriptor=prefix, | ||
date=target_date.strftime('%m/%d/%Y')) | ||
|
@@ -1238,14 +1238,13 @@ def process(self, value, is_value2): | |
return None | ||
|
||
if value is None: | ||
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 | ||
Comment on lines
-1241
to
+1247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
if self.op == ops.select_month: | ||
if is_value2: | ||
return feval.Int(not_empty=False, min=1900, max=9999).to_python(value) | ||
|
@@ -1429,14 +1428,13 @@ def process(self, value, is_value2): | |
return None | ||
|
||
if value is None: | ||
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 | ||
Comment on lines
-1432
to
+1437
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
if self.op == ops.select_month: | ||
if is_value2: | ||
return feval.Int(not_empty=False, min=1900, max=9999).to_python(value) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,11 +121,7 @@ def render(self): | |
class GroupMixin: | ||
def has_groups(self): | ||
"""Returns True if any of the renderer's columns is part of a column group.""" | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
def get_group_heading_colspans(self): | ||
"""Computes the number of columns spanned by various groups. | ||
|
@@ -392,9 +388,10 @@ def filtering_session_key(self): | |
|
||
def filtering_fields(self): | ||
"""Table rows for the filter area.""" | ||
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) | ||
] | ||
Comment on lines
-395
to
+394
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
rows = Markup('\n'.join(rows)) | ||
|
||
top_row = '' | ||
|
@@ -435,10 +432,7 @@ def filtering_col_label(self, col): | |
def filtering_col_op_select(self, col): | ||
"""Render select box for filter Operator options.""" | ||
filter = col.filter | ||
if not filter.is_display_active: | ||
current_selected = '' | ||
else: | ||
current_selected = filter.op | ||
current_selected = '' if not filter.is_display_active else filter.op | ||
Comment on lines
-438
to
+435
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function `HTML.filtering_col_op_selectz refactored with the following changes:
|
||
|
||
primary_op = filter.primary_op or filter.operators[0] | ||
is_primary = lambda op: 'primary' if op == primary_op else None # noqa: E731 | ||
|
@@ -840,9 +834,7 @@ def table_attrs(self, **kwargs): | |
|
||
def table_column_headings(self): | ||
"""Combine all rendered column headings and return as Markup.""" | ||
headings = [] | ||
for col in self.columns: | ||
headings.append(self.table_th(col)) | ||
headings = [self.table_th(col) for col in self.columns] | ||
Comment on lines
-843
to
+837
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
th_str = '\n'.join(headings) | ||
th_str = reindent(th_str, 12) | ||
return Markup(th_str) | ||
|
@@ -887,10 +879,7 @@ def table_th(self, col): | |
Sortable columns are rendered as links with the needed URL args.""" | ||
label = col.label | ||
if self.grid.sorter_on and col.can_sort: | ||
url_args = {} | ||
url_args['dgreset'] = None | ||
url_args['sort2'] = None | ||
url_args['sort3'] = None | ||
url_args = {'dgreset': None, 'sort2': None, 'sort3': None} | ||
Comment on lines
-890
to
+882
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
link_attrs = {} | ||
if self.grid.order_by and len(self.grid.order_by) == 1: | ||
current_sort, flag_desc = self.grid.order_by[0] | ||
|
@@ -953,10 +942,7 @@ def table_tr_styler(self, rownum, record): | |
row_hah = HTMLAttributes() | ||
|
||
# add odd/even classes to the rows | ||
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' | ||
Comment on lines
-956
to
+945
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
for styler in self.grid._rowstylers: | ||
styler(self.grid, rownum, row_hah, record) | ||
|
@@ -981,9 +967,7 @@ def table_tr(self, rownum, record): | |
row_hah = self.table_tr_styler(rownum, record) | ||
|
||
# get the <td>s for this row | ||
cells = [] | ||
for col in self.columns: | ||
cells.append(self.table_td(col, record)) | ||
cells = [self.table_td(col, record) for col in self.columns] | ||
Comment on lines
-984
to
+970
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
return self.table_tr_output(cells, row_hah) | ||
|
||
|
@@ -1008,11 +992,11 @@ def table_totals(self, rownum, record, label, numrecords): | |
'{label} ({num} records):', | ||
numrecords, | ||
label=label) | ||
buffer_hah = { | ||
'colspan': colspan, | ||
'class': 'totals-label' | ||
} | ||
if colspan: | ||
buffer_hah = { | ||
'colspan': colspan, | ||
'class': 'totals-label' | ||
} | ||
Comment on lines
-1011
to
+999
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
cells.append(self._render_jinja( | ||
'<td{{attrs|wg_attributes}}>{{val}}</td>', | ||
attrs=buffer_hah, | ||
|
@@ -1130,15 +1114,16 @@ def current_url(self, **kwargs): | |
|
||
def reset_url(self, session_reset=True): | ||
"""Generate a URL that will trigger a reset of the grid's UI options.""" | ||
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, | ||
} | ||
Comment on lines
-1133
to
+1126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
|
||
for col in six.itervalues(self.grid.filtered_cols): | ||
url_args['op({0})'.format(col.key)] = None | ||
|
@@ -1668,9 +1653,7 @@ def body_headings(self): | |
|
||
Note, column groups do not have real meaning in the CSV context, so | ||
they are left out here.""" | ||
headings = [] | ||
for col in self.columns: | ||
headings.append(col.label) | ||
headings = [col.label for col in self.columns] | ||
Comment on lines
-1671
to
+1656
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
self.writer.writerow(headings) | ||
|
||
def body_records(self): | ||
|
@@ -1683,9 +1666,7 @@ def body_records(self): | |
self.grid.set_paging(None, None) | ||
|
||
for rownum, record in enumerate(self.grid.records): | ||
row = [] | ||
for col in self.columns: | ||
row.append(col.render('csv', record)) | ||
row = [col.render('csv', record) for col in self.columns] | ||
Comment on lines
-1686
to
+1669
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Function
|
||
self.writer.writerow(row) | ||
|
||
def as_response(self): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function
Column.new_instance
refactored with the following changes: