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 Starbot ⭐ refactored lilydjwg/morerssplz #53
Conversation
if err_exc in (None, ' '): | ||
if (err_exc not in (None, ' ') and isinstance(err_exc, web.HTTPError) | ||
and err_exc.log_message is not None): | ||
err_msg = f'{str(err_exc.log_message)}.' | ||
elif (err_exc not in (None, ' ') and isinstance(err_exc, web.HTTPError) | ||
or err_exc in (None, ' ')): | ||
err_msg = '' | ||
else: | ||
if isinstance(err_exc, web.HTTPError): | ||
if err_exc.log_message is not None: | ||
err_msg = str(err_exc.log_message) + '.' | ||
else: | ||
err_msg = '' | ||
else: | ||
err_msg = str(err_exc) + '.' | ||
err_msg = f'{str(err_exc)}.' |
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 BaseHandler.write_error
refactored with the following changes:
- Merge duplicate blocks in conditional (
merge-duplicate-blocks
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Remove redundant conditional (
remove-redundant-if
)
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.
Too complex.
rss = PyRSS2Gen.RSS2( | ||
title = info['title'], | ||
link = url, | ||
lastBuildDate = datetime.datetime.now(), | ||
items = items, | ||
generator = 'morerssplz %s' % (__version__), | ||
description = info['description'], | ||
return PyRSS2Gen.RSS2( | ||
title=info['title'], | ||
link=url, | ||
lastBuildDate=datetime.datetime.now(), | ||
items=items, | ||
generator=f'morerssplz {__version__}', | ||
description=info['description'], | ||
) | ||
return rss |
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 data2rss
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
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.
A separate variable make it easy for logging and debugging.
url = 'ssl:' + url[8:] | ||
url = f'ssl:{url[8:]}' | ||
else: | ||
logger.error('bad image url: %s', url) | ||
url = url | ||
return 'https://images.weserv.nl/?url=%s' % url | ||
return f'https://images.weserv.nl/?url={url}' |
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 _proxify_url_cf
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
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.
Converting from str.__add__
to f-strings introduces potiential type issues: the former ensures both sides are str
s.
STATSC.timing('handler.%s.%s' % ( | ||
handler.__class__.__name__, code, | ||
), request_time) | ||
STATSC.timing(f'handler.{handler.__class__.__name__}.{code}', request_time) |
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 MyApp.log_request
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
description = content + '<br/><br/>' | ||
description = f'{content}<br/><br/>' | ||
|
||
if 'linkInfo' in post: | ||
linkInfo = post['linkInfo'] | ||
|
||
shareInfo = '链接分享:' | ||
if 'audio' in linkInfo: | ||
if linkInfo['audio'].get('subtype') == 'MUSIC': | ||
shareInfo = '音乐分享:' | ||
else: | ||
shareInfo = '未知分享:' | ||
|
||
shareInfo = '音乐分享:' if linkInfo['audio'].get('subtype') == 'MUSIC' 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 post2rss
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace if statement with if expression (
assign-if-exp
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
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.
Line too long.
url = 'https://www.v2ex.com/t/' + tid | ||
url = f'https://www.v2ex.com/t/{tid}' |
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 V2exCommentHandler.get
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
url = '%s#%s' % (url, rid) | ||
url = f'{url}#{rid}' | ||
content = comment.xpath('.//div[@class="reply_content"]')[0] | ||
author = comment.xpath('.//strong/a')[0].text | ||
content_text = content.text_content() | ||
if len(content_text) > 30: | ||
title = "%s 说: %s……" % (author, content_text[:30]) | ||
title = f"{author} 说: {content_text[:30]}……" | ||
else: | ||
title = "%s 说: %s" % (author, content_text) | ||
title = f"{author} 说: {content_text}" | ||
|
||
content = tostring(content, encoding=str).strip().replace('\r', '') | ||
|
||
item = PyRSS2Gen.RSSItem( | ||
return PyRSS2Gen.RSSItem( | ||
title = title, | ||
link = url, | ||
guid = url, | ||
description = content, | ||
author = author, | ||
) | ||
return item |
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 comment2rss
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if prev: | ||
prev = prev[0].get('href') | ||
else: | ||
prev = None | ||
|
||
prev = prev[0].get('href') if prev else 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 parse_webpage
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
'title': '[评论] %s' % data['subject'], | ||
'description': data['description'], | ||
'title': f"[评论] {data['subject']}", | ||
'description': data['description'], | ||
} | ||
rss = base.data2rss( | ||
return base.data2rss( | ||
url, | ||
rss_info, | ||
data['comments'], | ||
partial(comment2rss, url), | ||
) | ||
return rss |
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 test
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
baseurl = 'https://zhuanlan.zhihu.com/' + name | ||
baseurl = f'https://zhuanlan.zhihu.com/{name}' |
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 ZhihuZhuanlanHandler.get
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace call to format with f-string. (
use-fstring-for-formatting
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
info = json.loads(res.body.decode('utf-8')) | ||
return info | ||
return json.loads(res.body.decode('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 ZhihuZhuanlanHandler._get_url
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
elif article := article_from_cache(post['id'], post['updated']): | ||
# logger.debug('cache hit for %s', post['id']) | ||
base.STATSC.incr('zhihu.cache_hit') | ||
|
||
content = article['content'] | ||
if pic: | ||
doc = fromstring(content) | ||
base.proxify_pic(doc, zhihulib.re_zhihu_img, pic) | ||
content = tostring(doc, encoding=str) | ||
|
||
else: | ||
article = article_from_cache(post['id'], post['updated']) | ||
if not article: | ||
base.STATSC.incr('zhihu.cache_miss') | ||
try: | ||
_article_q.put_nowait(str(post['id'])) | ||
except asyncio.QueueFull: | ||
logger.warning('_article_q full') | ||
base.STATSC.incr('zhihu.queue_full') | ||
|
||
if fullonly: | ||
return None | ||
|
||
content = post['excerpt'] + ' (全文尚不可用)' | ||
content = zhihulib.process_content_for_html(content, pic=pic) | ||
|
||
else: | ||
# logger.debug('cache hit for %s', post['id']) | ||
base.STATSC.incr('zhihu.cache_hit') | ||
|
||
content = article['content'] | ||
if pic: | ||
doc = fromstring(content) | ||
base.proxify_pic(doc, zhihulib.re_zhihu_img, pic) | ||
content = tostring(doc, encoding=str) | ||
base.STATSC.incr('zhihu.cache_miss') | ||
try: | ||
_article_q.put_nowait(str(post['id'])) | ||
except asyncio.QueueFull: | ||
logger.warning('_article_q full') | ||
base.STATSC.incr('zhihu.queue_full') | ||
|
||
if fullonly: | ||
return None | ||
|
||
content = post['excerpt'] + ' (全文尚不可用)' | ||
content = zhihulib.process_content_for_html(content, pic=pic) | ||
|
||
if post.get('title_image'): | ||
content = '<p><img src="%s"></p>' % post['title_image'] + content | ||
|
||
item = PyRSS2Gen.RSSItem( | ||
return PyRSS2Gen.RSSItem( |
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 post2rss
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Use named expression to simplify assignment and conditional (
use-named-expression
) - Swap if/else branches (
swap-if-else-branches
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
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.
The two layers of if
checks distinct aspects of things and should not be merged.
Swapping the if
is undesired because when you get data from cache, you first think what if it's not there before continuing.
url = 'https://zhuanlan.zhihu.com/api/columns/' + column | ||
url = f'https://zhuanlan.zhihu.com/api/columns/{column}' | ||
info = s.get(url).json() | ||
url = 'https://zhuanlan.zhihu.com/api/columns/%s/posts' % column | ||
url = f'https://zhuanlan.zhihu.com/api/columns/{column}/posts' | ||
posts = s.get(url).json() | ||
|
||
rss_info = { | ||
'title': '%s - 知乎专栏' % info['name'], | ||
'description': info.get('description', ''), | ||
'title': f"{info['name']} - 知乎专栏", | ||
'description': info.get('description', ''), | ||
} | ||
rss = base.data2rss( | ||
return base.data2rss( | ||
baseurl, | ||
rss_info, posts, | ||
partial(post2rss, url, pic='cf'), | ||
) | ||
return rss |
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 test
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
url = 'moments/%s/activities' % name | ||
url = f'moments/{name}/activities' | ||
query = { | ||
'desktop': 'True', | ||
'after_id': str(int(time.time())), | ||
'limit': '7', | ||
} | ||
url += '?' + urlencode(query) | ||
url += f'?{urlencode(query)}' |
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 ZhihuAPI.activities
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
url = 'members/%s/pins/' % name | ||
url = f'members/{name}/pins/' | ||
query = { | ||
'desktop': 'True', | ||
'after_id': str(int(time.time())), | ||
'limit': '7', | ||
} | ||
url += '?' + urlencode(query) | ||
url += f'?{urlencode(query)}' |
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 ZhihuAPI.pins
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
url = 'https://api.zhihu.com/questions/%s?include=detail' % id | ||
url = f'https://api.zhihu.com/questions/{id}?include=detail' |
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 ZhihuAPI.question_info
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
info = { | ||
'title': '%s - 知乎动态' % info['name'], | ||
'description': info['headline'], | ||
} | ||
info = {'title': f"{info['name']} - 知乎动态", 'description': info['headline']} |
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 activities2rss
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Replace identity comprehension with call to collection constructor (
identity-comprehension
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
info = { | ||
'title': '%s - 知乎赞同' % info['name'], | ||
'description': info['headline'], | ||
} | ||
info = {'title': f"{info['name']} - 知乎赞同", 'description': info['headline']} |
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 upvote2rss
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if not origin_pin['is_deleted']: | ||
merged_content += pin_content(origin_pin) | ||
else: | ||
merged_content += origin_pin['deleted_reason'] | ||
|
||
merged_content += (origin_pin['deleted_reason'] if origin_pin['is_deleted'] | ||
else pin_content(origin_pin)) |
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 pin_content
refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression
) - Replace if statement with if expression (
assign-if-exp
)
content = post['title'] | ||
elif digest: | ||
content = post['excerpt'] | ||
# Posts in Zhihu topics API response don't have the 'content' key by default | ||
# Although they can include it by carrying verbose query params | ||
# which are hard to maintain because Zhihu doesn't have public API documentation :( | ||
elif 'content' not in post: | ||
content = post['excerpt'] | ||
return post['title'] | ||
elif digest or 'content' not in post: | ||
return post['excerpt'] | ||
else: | ||
content = post['content'] | ||
|
||
return content | ||
return post['content'] |
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 post_content
refactored with the following changes:
- Merge duplicate blocks in conditional (
merge-duplicate-blocks
) - Remove redundant conditional (
remove-redundant-if
) - Lift return into if (
lift-return-into-if
)
This removes the following comments ( why? ):
# Posts in Zhihu topics API response don't have the 'content' key by default
# Although they can include it by carrying verbose query params
# which are hard to maintain because Zhihu doesn't have public API documentation :(
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.
This bot knows syntax and grammar quite well, but totally ignores engineering and business logic. In short, it's not worth messing with it.
if err_exc in (None, ' '): | ||
if (err_exc not in (None, ' ') and isinstance(err_exc, web.HTTPError) | ||
and err_exc.log_message is not None): | ||
err_msg = f'{str(err_exc.log_message)}.' | ||
elif (err_exc not in (None, ' ') and isinstance(err_exc, web.HTTPError) | ||
or err_exc in (None, ' ')): | ||
err_msg = '' | ||
else: | ||
if isinstance(err_exc, web.HTTPError): | ||
if err_exc.log_message is not None: | ||
err_msg = str(err_exc.log_message) + '.' | ||
else: | ||
err_msg = '' | ||
else: | ||
err_msg = str(err_exc) + '.' | ||
err_msg = f'{str(err_exc)}.' |
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.
Too complex.
rss = PyRSS2Gen.RSS2( | ||
title = info['title'], | ||
link = url, | ||
lastBuildDate = datetime.datetime.now(), | ||
items = items, | ||
generator = 'morerssplz %s' % (__version__), | ||
description = info['description'], | ||
return PyRSS2Gen.RSS2( | ||
title=info['title'], | ||
link=url, | ||
lastBuildDate=datetime.datetime.now(), | ||
items=items, | ||
generator=f'morerssplz {__version__}', | ||
description=info['description'], | ||
) | ||
return rss |
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.
A separate variable make it easy for logging and debugging.
url = 'ssl:' + url[8:] | ||
url = f'ssl:{url[8:]}' | ||
else: | ||
logger.error('bad image url: %s', url) | ||
url = url | ||
return 'https://images.weserv.nl/?url=%s' % url | ||
return f'https://images.weserv.nl/?url={url}' |
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.
Converting from str.__add__
to f-strings introduces potiential type issues: the former ensures both sides are str
s.
description = content + '<br/><br/>' | ||
description = f'{content}<br/><br/>' | ||
|
||
if 'linkInfo' in post: | ||
linkInfo = post['linkInfo'] | ||
|
||
shareInfo = '链接分享:' | ||
if 'audio' in linkInfo: | ||
if linkInfo['audio'].get('subtype') == 'MUSIC': | ||
shareInfo = '音乐分享:' | ||
else: | ||
shareInfo = '未知分享:' | ||
|
||
shareInfo = '音乐分享:' if linkInfo['audio'].get('subtype') == 'MUSIC' 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.
Line too long.
item = article2rssitem(edge) | ||
elif typename == 'comment' or typename == 'broadcast': | ||
item = comment2rssitem(edge) | ||
return article2rssitem(edge) | ||
elif typename in ['comment', 'broadcast']: | ||
return comment2rssitem(edge) | ||
else: | ||
item = PyRSS2Gen.RSSItem( | ||
return PyRSS2Gen.RSSItem( |
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.
Returning from multiple branches introduces bugs easily and prevent adding code operating on the returning value..
elif article := article_from_cache(post['id'], post['updated']): | ||
# logger.debug('cache hit for %s', post['id']) | ||
base.STATSC.incr('zhihu.cache_hit') | ||
|
||
content = article['content'] | ||
if pic: | ||
doc = fromstring(content) | ||
base.proxify_pic(doc, zhihulib.re_zhihu_img, pic) | ||
content = tostring(doc, encoding=str) | ||
|
||
else: | ||
article = article_from_cache(post['id'], post['updated']) | ||
if not article: | ||
base.STATSC.incr('zhihu.cache_miss') | ||
try: | ||
_article_q.put_nowait(str(post['id'])) | ||
except asyncio.QueueFull: | ||
logger.warning('_article_q full') | ||
base.STATSC.incr('zhihu.queue_full') | ||
|
||
if fullonly: | ||
return None | ||
|
||
content = post['excerpt'] + ' (全文尚不可用)' | ||
content = zhihulib.process_content_for_html(content, pic=pic) | ||
|
||
else: | ||
# logger.debug('cache hit for %s', post['id']) | ||
base.STATSC.incr('zhihu.cache_hit') | ||
|
||
content = article['content'] | ||
if pic: | ||
doc = fromstring(content) | ||
base.proxify_pic(doc, zhihulib.re_zhihu_img, pic) | ||
content = tostring(doc, encoding=str) | ||
base.STATSC.incr('zhihu.cache_miss') | ||
try: | ||
_article_q.put_nowait(str(post['id'])) | ||
except asyncio.QueueFull: | ||
logger.warning('_article_q full') | ||
base.STATSC.incr('zhihu.queue_full') | ||
|
||
if fullonly: | ||
return None | ||
|
||
content = post['excerpt'] + ' (全文尚不可用)' | ||
content = zhihulib.process_content_for_html(content, pic=pic) | ||
|
||
if post.get('title_image'): | ||
content = '<p><img src="%s"></p>' % post['title_image'] + content | ||
|
||
item = PyRSS2Gen.RSSItem( | ||
return PyRSS2Gen.RSSItem( |
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.
The two layers of if
checks distinct aspects of things and should not be merged.
Swapping the if
is undesired because when you get data from cache, you first think what if it's not there before continuing.
Thanks for starring sourcery-ai/sourcery ✨ 🌟 ✨
Here's your pull request refactoring your most popular Python repo.
If you want Sourcery to refactor all your Python repos and incoming pull requests install our bot.
Review changes via command line
To manually merge these changes, make sure you're on the
master
branch, then run: