Skip to content
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 refactored master branch #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

sourcery-ai[bot]
Copy link

@sourcery-ai sourcery-ai bot commented Jul 27, 2020

Branch master refactored 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 master branch, then run:

git fetch origin sourcery/master
git merge --ff-only FETCH_HEAD
git reset HEAD^

Comment on lines -92 to +93
for e in exclude:
if exclude[e] is not None:
for e, value in exclude.items():
if value is not None:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 92-93 refactored with the following changes:

  • Use items() to directly unpack dictionary values (use-dict-items)

Comment on lines -31 to +45
if change and settings.ACCOUNTS_APPROVAL_REQUIRED:
if obj.is_active and not User.objects.get(id=obj.id).is_active:
if settings.ACCOUNTS_VERIFICATION_REQUIRED:
# Accounts verification requires an inactive account
obj.is_active = False
# The token generated by send_verification_mail()
# must match the _saved_ User object,
# so postpone send_verification_mail() until later
must_send_verification_mail_after_save = True
else:
send_approved_mail(request, obj)
if (
change
and settings.ACCOUNTS_APPROVAL_REQUIRED
and obj.is_active
and not User.objects.get(id=obj.id).is_active
):
if settings.ACCOUNTS_VERIFICATION_REQUIRED:
# Accounts verification requires an inactive account
obj.is_active = False
# The token generated by send_verification_mail()
# must match the _saved_ User object,
# so postpone send_verification_mail() until later
must_send_verification_mail_after_save = True
else:
send_approved_mail(request, obj)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function UserProfileAdmin.save_model refactored with the following changes:

  • Merge nested if conditions (merge-nested-ifs)

user_fields = set([f.name for f in User._meta.get_fields()])
user_fields = {f.name for f in User._meta.get_fields()}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function ProfileForm.__init__ refactored with the following changes:

  • Replace unneeded comprehension with generator (comprehension-to-generator)
  • Replace list(), dict() or set() with comprehension (collection-builtin-to-comprehension)

Comment on lines -58 to +61
for (name, items) in settings.ADMIN_MENU_ORDER:
if "blog.BlogCategory" in items:
return True
return False
return any(
"blog.BlogCategory" in items
for (name, items) in settings.ADMIN_MENU_ORDER
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function BlogCategoryAdmin.has_module_permission refactored with the following changes:

  • Use any() instead of for loop (use-any)

Comment on lines -52 to +53
post_dict = dict()
post_dict["title"] = post_element.find("blogml:title",
namespaces=ns).text
post_dict = {"title": post_element.find("blogml:title",
namespaces=ns).text}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Command.handle_import._process_post_element refactored with the following changes:

  • Replace dict() with {} (dict-literal)
  • Merge dictionary assignment with declaration (merge-dict-assign)
  • Replace list(), dict() or set() with comprehension (collection-builtin-to-comprehension)

formfield = super(RichTextField, self).formfield(**kwargs)
return formfield
return super(RichTextField, self).formfield(**kwargs)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function RichTextField.formfield refactored with the following changes:

  • Inline variable that is immediately returned (inline-immediately-returned-variable)

count = count / age**settings.SEARCH_AGE_SCALE_FACTOR
count /= age**settings.SEARCH_AGE_SCALE_FACTOR
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function SearchableQuerySet.annotate_scores refactored with the following changes:

  • Replace assignment with augmented assignment (aug-assign)

Comment on lines -272 to +277
search_fields = []
for f in self.model._meta.get_fields():
if isinstance(f, (CharField, TextField)):
search_fields.append(f.name)
search_fields = [
f.name
for f in self.model._meta.get_fields()
if isinstance(f, (CharField, TextField))
]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function SearchableManager.get_search_fields refactored with the following changes:

  • Convert for loop into list comprehension (list-comprehension)

Comment on lines -206 to +224
if (cache_installed() and request.method == "GET" and
not is_authenticated(request.user)):
cache_key = cache_key_prefix(request) + request.get_full_path()
response = cache_get(cache_key)
# We need to force a csrf token here, as new sessions
# won't receieve one on their first request, with cache
# middleware running.
if csrf_middleware_installed():
csrf_mw = CsrfViewMiddleware()
csrf_mw.process_view(request, lambda x: None, None, None)
get_token(request)
if response is None:
request._update_cache = True
else:
return HttpResponse(response)
if (
not cache_installed()
or request.method != "GET"
or is_authenticated(request.user)
):
return
cache_key = cache_key_prefix(request) + request.get_full_path()
response = cache_get(cache_key)
# We need to force a csrf token here, as new sessions
# won't receieve one on their first request, with cache
# middleware running.
if csrf_middleware_installed():
csrf_mw = CsrfViewMiddleware()
csrf_mw.process_view(request, lambda x: None, None, None)
get_token(request)
if response is None:
request._update_cache = True
else:
return HttpResponse(response)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function FetchFromCacheMiddleware.process_request refactored with the following changes:

  • Add guard clause (last-if-guard)

warnings = []
warnings.extend(run_pyflakes_for_package("mezzanine"))
warnings = list(run_pyflakes_for_package("mezzanine"))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function CoreTests.test_syntax refactored with the following changes:

  • Merge extend into list declaration (merge-list-extend)

if name in template_src:
prev = ' [copied from %s]' % template_src[name]
else:
prev = ''
prev = ' [copied from %s]' % template_src[name] if name in template_src else ''
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Command.handle refactored with the following changes:

  • Replace if statement with if expression (assign-if-exp)

self.assertTrue(all([image.description for image in images]))
self.assertTrue(all(image.description for image in images))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function GalleriesTests.test_gallery_import refactored with the following changes:

  • Replace unneeded comprehension with generator (comprehension-to-generator)

Comment on lines -214 to +216
string_field_name = list(self.fields.keys())[0] % \
self.related_field_name
if hasattr(cls, "search_fields") and name in cls.search_fields:
string_field_name = list(self.fields.keys())[0] % \
self.related_field_name
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function KeywordsField.contribute_to_class refactored with the following changes:

  • Move assignments closer to their usage (move-assign)

if callable(self.default):
default = self.default()
else:
default = self.default
default = self.default() if callable(self.default) else self.default
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function MenusField.get_default refactored with the following changes:

  • Replace if statement with if expression (assign-if-exp)

Comment on lines -97 to +98
if self.__class__ == Page:
if self.content_model:
return self.get_content_model().description_from_content()
if self.__class__ == Page and self.content_model:
return self.get_content_model().description_from_content()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function Page.description_from_content refactored with the following changes:

  • Merge nested if conditions (merge-nested-ifs)

Comment on lines -54 to +61
if isinstance(setting_default, str):
if gethostname() in setting_default or (
setting_default.startswith("/") and
os.path.exists(setting_default)):
setting_default = dynamic
if isinstance(setting_default, str) and (
gethostname() in setting_default
or (
setting_default.startswith("/")
and os.path.exists(setting_default)
)
):
setting_default = dynamic
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function build_settings_docs refactored with the following changes:

  • Merge nested if conditions (merge-nested-ifs)

Comment on lines -212 to +221
if hotfix or entry not in versions[version]["changes"]:
if hotfix:
versions[hotfix] = {
"changes": [entry],
"date": _changeset_date(cs).strftime("%b %d, %Y"),
}
else:
versions[version]["changes"].insert(0, entry)
if hotfix:
versions[hotfix] = {
"changes": [entry],
"date": _changeset_date(cs).strftime("%b %d, %Y"),
}
elif entry not in versions[version]["changes"]:
versions[version]["changes"].insert(0, entry)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function build_changelog refactored with the following changes:

  • Split conditional into multiple branches (split-or-ifs)
  • Remove redundant conditional (remove-redundant-if)

@@ -289,14 +291,14 @@ def build_requirements(docs_path, package_name="mezzanine"):
"""
Updates the requirements file with Mezzanine's version number.
"""
mezz_string = "Mezzanine=="
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function build_requirements refactored with the following changes:

  • Move assignments closer to their usage (move-assign)

Comment on lines -56 to +64
if not site_id:
try:
site = Site.objects.get(domain__iexact=domain)
except Site.DoesNotExist:
pass
else:
site_id = site.id
if cache_installed():
cache_set(cache_key, site_id)
if not site_id:
try:
site = Site.objects.get(domain__iexact=domain)
except Site.DoesNotExist:
pass
else:
site_id = site.id
if cache_installed():
cache_set(cache_key, site_id)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function current_site_id refactored with the following changes:

  • Hoist conditional out of nested conditional (hoist-if-from-if)

if os.path.isdir(test_path):
copy = copytree
else:
copy = copyfile
copy = copytree if os.path.isdir(test_path) else copyfile
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function copy_test_to_media refactored with the following changes:

  • Replace if statement with if expression (assign-if-exp)

@sourcery-ai
Copy link
Author

sourcery-ai bot commented Jul 27, 2020

Sourcery Code Quality Report (beta)

✅  Merging this PR will increase code quality in the affected files by 0.02 out of 10.

Quality metrics Before After Change
Complexity 3.20 3.06 -0.14 🔵
Method Length 62.04 61.72 -0.32 🔵
Quality 8.40 8.42 0.02 🔵
Other metrics Before After Change
Lines 5040 5044 4
Changed files Quality Before Quality After Quality Change
setup.py 4.22 4.22 0.00
mezzanine/accounts/admin.py 8.67 8.81 0.14 🔵
mezzanine/accounts/forms.py 8.00 8.00 0.00
mezzanine/blog/admin.py 8.87 8.96 0.09 🔵
mezzanine/blog/management/commands/import_blogml.py 8.07 8.18 0.11 🔵
mezzanine/blog/management/commands/import_posterous.py 7.36 7.38 0.02 🔵
mezzanine/blog/management/commands/import_tumblr.py 8.90 8.90 0.00
mezzanine/blog/management/commands/import_wordpress.py 7.61 7.61 0.00
mezzanine/conf/init.py 8.73 8.75 0.02 🔵
mezzanine/conf/context_processors.py 9.32 9.32 0.00
mezzanine/conf/forms.py 7.03 7.10 0.07 🔵
mezzanine/core/admin.py 8.71 8.72 0.01 🔵
mezzanine/core/auth_backends.py 7.66 7.89 0.23 🔵
mezzanine/core/fields.py 9.21 9.21 0.00
mezzanine/core/managers.py 8.17 8.20 0.03 🔵
mezzanine/core/middleware.py 7.97 7.99 0.02 🔵
mezzanine/core/tests.py 8.63 8.63 0.00
mezzanine/core/management/commands/collecttemplates.py 6.62 6.64 0.02 🔵
mezzanine/galleries/tests.py 8.31 8.31 0.00
mezzanine/generic/fields.py 8.48 8.48 0.00
mezzanine/pages/fields.py 9.27 9.29 0.02 🔵
mezzanine/pages/models.py 8.93 8.93 0.00
mezzanine/pages/tests.py 8.29 8.29 0.00
mezzanine/pages/views.py 7.62 7.62 0.00
mezzanine/twitter/models.py 8.15 8.15 0.00
mezzanine/utils/docs.py 6.62 6.65 0.03 🔵
mezzanine/utils/sites.py 8.41 8.46 0.05 🔵
mezzanine/utils/tests.py 8.49 8.50 0.01 🔵

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Overall Recommendation
mezzanine/utils/docs.py build_changelog 70 610.03 0.60 Split out functionality. Reduce complexity
mezzanine/core/management/commands/collecttemplates.py Command.handle 51 341.58 1.47 Split out functionality. Reduce complexity
mezzanine/blog/management/commands/import_posterous.py Command.handle_import 24 288.56 3.30 Split out functionality
mezzanine/twitter/models.py Query.run 21 354.58 3.30 Split out functionality
mezzanine/core/managers.py SearchableManager.search 23 226.91 3.79 Split out functionality

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!

Let us know what you think of it via email or our Gitter!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants