-
Notifications
You must be signed in to change notification settings - Fork 10
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
Feature/lintering #27
Conversation
Codecov Report
@@ Coverage Diff @@
## main #27 +/- ##
==========================================
- Coverage 93.67% 92.55% -1.13%
==========================================
Files 44 44
Lines 2008 2014 +6
Branches 171 200 +29
==========================================
- Hits 1881 1864 -17
- Misses 107 129 +22
- Partials 20 21 +1
Continue to review full report at Codecov.
|
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.
I made some comments inline and pushed some commits.
For imports under tests/
, I order imports in a certain way: general modules, test-related modules (e.g., pytest, mock, and lastly from atpbar. Is it possible to configure isort in this way? Or otherwise can you not use isort
on test files?
from ._version import get_versions | ||
__version__ = get_versions()['version'] | ||
from .funcs import disable, find_reporter, flush, register_reporter | ||
from .main import atpbar | ||
|
||
__version__ = get_versions()["version"] | ||
del get_versions |
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.
Can you revert these lines to the original order?
The get_versions
is used in this file and deleted while other imported objects are there to be imported by users' scripts.
@@ -1,4 +1,3 @@ | |||
|
|||
# This file helps to compute a version number in source trees obtained from |
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.
Please do not modify this file. This file is part of Versioneer is not managed in this repo.
'ipywidgets>=7.5', | ||
] | ||
} | ||
"lintering": [ |
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.
Please call this dev
instead of lintering
.
pytest.param( | ||
dict(taskid=1, last=False), | ||
[ ], [ ], [ ], [ ], | ||
[1], [ ], [ ], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=False), | ||
[1], [ ], [ ], [ ], | ||
[1], [ ], [ ], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=False), | ||
[ ], [1], [ ], [ ], | ||
[ ], [1], [ ], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=False), | ||
[ ], [ ], [1], [ ], | ||
[ ], [ ], [1], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=False), | ||
[ ], [ ], [ ], [1], | ||
[ ], [ ], [ ], [1], | ||
False | ||
), | ||
|
||
pytest.param(dict(taskid=1, last=False), [], [], [], [], [1], [], [], [], True), | ||
pytest.param(dict(taskid=1, last=False), [1], [], [], [], [1], [], [], [], True), | ||
pytest.param(dict(taskid=1, last=False), [], [1], [], [], [], [1], [], [], True), | ||
pytest.param(dict(taskid=1, last=False), [], [], [1], [], [], [], [1], [], True), | ||
pytest.param(dict(taskid=1, last=False), [], [], [], [1], [], [], [], [1], False), | ||
## | ||
pytest.param( | ||
dict(taskid=1, last=True), | ||
[ ], [ ], [ ], [ ], | ||
[ ], [ ], [1], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=True), | ||
[1], [ ], [ ], [ ], | ||
[ ], [ ], [1], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=True), | ||
[ ], [1], [ ], [ ], | ||
[ ], [ ], [1], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=True), | ||
[ ], [ ], [1], [ ], | ||
[ ], [ ], [1], [ ], | ||
True | ||
), | ||
pytest.param( | ||
dict(taskid=1, last=True), | ||
[ ], [ ], [ ], [1], | ||
[ ], [ ], [ ], [1], | ||
False | ||
), |
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.
Please don't change these lines. You can add #fmt: on/off
[ ], [ ], [ ], [ ], | ||
[ ], [ ], [ ], [ ], | ||
[], | ||
[], | ||
[], | ||
[], | ||
[], | ||
[], | ||
[], | ||
[], | ||
), | ||
pytest.param( | ||
[ ], [10, 11, 12], [ ], [30, 31, 32], | ||
[ ], [10, 11, 12], [ ], [30, 31, 32], | ||
[], | ||
[10, 11, 12], | ||
[], | ||
[30, 31, 32], | ||
[], | ||
[10, 11, 12], | ||
[], | ||
[30, 31, 32], | ||
), | ||
pytest.param( | ||
[1, 2], [10, 11, 12], [20, 21], [30, 31, 32], | ||
[ ], [10, 11, 12, 1, 2], [ ], [30, 31, 32, 20, 21], | ||
[1, 2], | ||
[10, 11, 12], | ||
[20, 21], | ||
[30, 31, 32], | ||
[], | ||
[10, 11, 12, 1, 2], | ||
[], | ||
[30, 31, 32, 20, 21], |
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.
Please don't change these lines. You can add #fmt: on/off
}) | ||
module = sys.modules['atpbar.presentation.create'] | ||
monkeypatch.setattr(module, 'sys', f) | ||
f = mock.Mock(**{"stdout.isatty.return_value": ret, "stdout.write.side_effect": lambda x: org_stdout.write(x)}) |
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.
Can you keep this as multiple lines? If you add a comma (,
) after org_stdout.write(x)
, Black will keep it as multiple lines.
Running black and isort on the entire code base. This could be automated in future, e.g. by using tox or gh actions (in order to validate each commit / PR)
This resolves #26