-
Notifications
You must be signed in to change notification settings - Fork 3
Sourcery refactored master branch #1
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
Conversation
|
|
||
| def make_user_key(self, username): | ||
| return 'user_{}'.format(username) | ||
| return f'user_{username}' |
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 UserResource.make_user_key refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| users.append(self.conn.hgetall(self.make_user_key(user))) | ||
|
|
||
| return users | ||
| return [self.conn.hgetall(self.make_user_key(user)) for user in usernames] |
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 UserResource.list refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
|
|
||
| def make_user_key(self, username): | ||
| return 'user_{}'.format(username) | ||
| return f'user_{username}' |
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 UserResource.make_user_key refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| users.append(self.conn.hgetall(self.make_user_key(user))) | ||
|
|
||
| return users | ||
| return [self.conn.hgetall(self.make_user_key(user)) for user in usernames] |
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 UserResource.list refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
| def as_list(self, *args, **kwargs): | ||
| return csrf_exempt(super(DjangoResource, self).as_list(*args, **kwargs)) | ||
| def as_list(cls, *args, **kwargs): | ||
| return csrf_exempt(super(DjangoResource, cls).as_list(*args, **kwargs)) |
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 DjangoResource.as_list refactored with the following changes:
- The first argument to class methods should be
cls(class-method-first-arg-name)
| # ``endpoint`` errors as well. | ||
| if not method in self.http_methods.get(endpoint, {}): | ||
| if method not in self.http_methods.get(endpoint, {}): | ||
| raise MethodNotImplemented( | ||
| "Unsupported method '{}' for {} endpoint.".format( | ||
| method, | ||
| endpoint | ||
| ) | ||
| f"Unsupported method '{method}' for {endpoint} endpoint." | ||
| ) | ||
|
|
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 Resource.handle refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan) - Replace call to format with f-string (
use-fstring-for-formatting)
| if body: | ||
| return self.serializer.deserialize(body) | ||
|
|
||
| return [] | ||
| return self.serializer.deserialize(body) if body else [] |
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 Resource.deserialize_list refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| if body: | ||
| return self.serializer.deserialize(body) | ||
|
|
||
| return {} | ||
| return self.serializer.deserialize(body) if body else {} |
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 Resource.deserialize_detail refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
| if not getattr(data, 'should_prepare', True): | ||
| prepped_data = data.value | ||
| else: | ||
| prepped_data = [self.prepare(item) for item in data] | ||
| prepped_data = ( | ||
| [self.prepare(item) for item in data] | ||
| if getattr(data, 'should_prepare', True) | ||
| else data.value | ||
| ) |
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 Resource.serialize_list refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp) - Swap if/else branches of if expression to remove negation (
swap-if-expression)
| if not getattr(data, 'should_prepare', True): | ||
| prepped_data = data.value | ||
| else: | ||
| prepped_data = self.prepare(data) | ||
| prepped_data = ( | ||
| self.prepare(data) | ||
| if getattr(data, 'should_prepare', True) | ||
| else data.value | ||
| ) |
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 Resource.serialize_detail refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp) - Swap if/else branches of if expression to remove negation (
swap-if-expression)
| if self.request_method() == 'GET': | ||
| return True | ||
|
|
||
| return False | ||
| return self.request_method() == 'GET' |
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 Resource.is_authenticated refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp) - Simplify boolean if expression (
boolean-if-exp-identity) - Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast)
| else: | ||
| FUTURES = (Future, futures.Future) | ||
|
|
||
| FUTURES = Future if futures is None else (Future, futures.Future) |
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.
Lines 27-31 refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
| new_cls = type( | ||
| cls.__name__ + '_' + _BridgeMixin.__name__ + '_restless', | ||
| (_BridgeMixin, cls._request_handler_base_,), | ||
| f'{cls.__name__}_{_BridgeMixin.__name__}_restless', | ||
| ( | ||
| _BridgeMixin, | ||
| cls._request_handler_base_, | ||
| ), | ||
| dict( | ||
| __resource_cls__=cls, | ||
| __resource_args__=init_args, | ||
| __resource_kwargs__=init_kwargs, | ||
| __resource_view_type__=view_type) | ||
| __resource_view_type__=view_type, | ||
| ), | ||
| ) | ||
|
|
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 TornadoResource.as_view refactored with the following changes:
- Use f-string instead of string concatenation [×3] (
use-fstring-for-concatenation) - Replace a[0:x] with a[:x] and a[x:len(a)] with a[x:] (
remove-redundant-slice-index)
| if status == NO_CONTENT: | ||
| # Avoid crashing the client when it tries to parse nonexisting JSON. | ||
| content_type = 'text/plain' | ||
| else: | ||
| content_type = 'application/json' | ||
| self.ref_rh.set_header("Content-Type", "{}; charset=UTF-8" | ||
| .format(content_type)) | ||
| content_type = 'text/plain' if status == NO_CONTENT else 'application/json' | ||
| self.ref_rh.set_header("Content-Type", f"{content_type}; charset=UTF-8") |
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 TornadoResource.build_response refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp) - Replace call to format with f-string (
use-fstring-for-formatting)
This removes the following comments ( why? ):
# Avoid crashing the client when it tries to parse nonexisting JSON.
| try: | ||
| if not method in self.http_methods.get(endpoint, {}): | ||
| if method not in self.http_methods.get(endpoint, {}): | ||
| raise MethodNotImplemented( | ||
| "Unsupported method '{}' for {} endpoint.".format( | ||
| method, | ||
| endpoint | ||
| ) | ||
| f"Unsupported method '{method}' for {endpoint} endpoint." | ||
| ) | ||
|
|
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 TornadoResource.handle refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan) - Replace call to format with f-string (
use-fstring-for-formatting)
| raise ObjectDoesNotExist("Model with pk {} not found.".format(pk)) | ||
| raise ObjectDoesNotExist(f"Model with pk {pk} not found.") |
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 DjTestResource.detail refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| raise Http404("Model with pk {} not found.".format(pk)) | ||
| raise Http404(f"Model with pk {pk} not found.") |
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 DjTestResourceHttp404Handling.detail refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| return False | ||
|
|
||
| return True | ||
| return self.request_method() != 'DELETE' |
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 PyrTestResource.is_authenticated refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp) - Simplify boolean if expression (
boolean-if-exp-identity) - Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast)
| for i in six.moves.xrange(min(len(v), len(version_info))): | ||
| if v[i] != version_info[i]: | ||
| return False | ||
| return True | ||
| return all( | ||
| v[i] == version_info[i] | ||
| for i in six.moves.xrange(min(len(v), len(version_info))) | ||
| ) |
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 _equal_ refactored with the following changes:
- Use any() instead of for loop (
use-any) - Invert any/all to simplify comparisons (
invert-any-all)
| if item['id'] == pk: | ||
| return item | ||
| return None | ||
| return next((item for item in self.fake_db if item['id'] == pk), None) |
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 TndBasicTestResource.detail refactored with the following changes:
- Use the built-in function
nextinstead of a for-loop (use-next)
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 0.25%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
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.
Worth considering. View full project report here.
| def as_list(self, *args, **kwargs): | ||
| return csrf_exempt(super(DjangoResource, self).as_list(*args, **kwargs)) | ||
| def as_list(cls, *args, **kwargs): | ||
| return csrf_exempt(super(DjangoResource, cls).as_list(*args, **kwargs)) |
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.
| return csrf_exempt(super(DjangoResource, cls).as_list(*args, **kwargs)) | |
| return csrf_exempt(super().as_list(*args, **kwargs)) |
It's unnecessary to use arguments when calling super for the parent class. Read more.
| def as_detail(self, *args, **kwargs): | ||
| return csrf_exempt(super(DjangoResource, self).as_detail(*args, **kwargs)) | ||
| def as_detail(cls, *args, **kwargs): | ||
| return csrf_exempt(super(DjangoResource, cls).as_detail(*args, **kwargs)) |
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.
| return csrf_exempt(super(DjangoResource, cls).as_detail(*args, **kwargs)) | |
| return csrf_exempt(super().as_detail(*args, **kwargs)) |
As above, These super arguments are unnecessary.
Branch
masterrefactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
masterbranch, then run:Help us improve this pull request!