-
Notifications
You must be signed in to change notification settings - Fork 477
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
Apply pyupgrade to project #2364
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 |
---|---|---|
@@ -1,5 +1,3 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
import glob | ||
import os.path as op | ||
import os | ||
|
@@ -41,9 +39,9 @@ | |
|
||
def test_dictionaries_exist(): | ||
"""Test consistency of dictionaries.""" | ||
doc_fnames = set(op.basename(f[0]) for f in _fnames_in_aspell) | ||
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. This change kinda makes it less readable - have to look into the expression to see if it's a dict or set. 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. I suppose that is a matter of familiarity. Lots of Python projects I've worked on use both set and dict comprehension. Once the syntax is understood and familiar it is readable. If the project has a policy against this syntax, let me know and I can revert those lines. 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. The Perhaps the
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. I would revert these two lines in this merge request. Let's discuss them in #2085. |
||
got_fnames = set(op.basename(f) | ||
for f in glob.glob(op.join(_data_dir, '*.txt'))) | ||
doc_fnames = {op.basename(f[0]) for f in _fnames_in_aspell} | ||
got_fnames = {op.basename(f) | ||
for f in glob.glob(op.join(_data_dir, '*.txt'))} | ||
assert doc_fnames == got_fnames | ||
|
||
|
||
|
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.
Not sure about this one either. What's wrong with the explicit
'r'
? It echoes the'w'
below.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.
'r'
is the default, so is unnecessary:https://docs.python.org/3/library/functions.html#open
It is conventional within the larger Python community to omit it. The goal of this PR is to update the code to follow wider community conventions.