-
Notifications
You must be signed in to change notification settings - Fork 90
Basic linter config #261
Basic linter config #261
Conversation
|
Hi @hanneshapke, I'm not feeling well and might not be able to take a look at your PR this week, so I requested additional reviewers. |
|
Thank you, @codesue. Get well soon! |
| for item in subfield_json_value: | ||
| if isinstance(item, dict): | ||
| new_object = field.__annotations__[subfield_key].__args__[0]() # pytype: disable=attribute-error | ||
| new_object = \ |
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.
Could we disable line length here? This is hard to read.
model_card_toolkit/core.py
Outdated
| if eval_result: | ||
| logging.info('EvalResult found at path %s', eval_result_path) | ||
| if self._source.tfma.metrics_include or self._source.tfma.metrics_exclude: | ||
| if self._source.tfma.metrics_include or \ |
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.
Could we use parentheses instead of \ here and elsewhere?
tools/build_api_docs.py
Outdated
| "output_dir", | ||
| default="/tmp/model_card_toolkit", | ||
| help="Where to output the docs") | ||
| flags.DEFINE_string("output_dir", |
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 personally find the original way of putting the arguments on a new line easier to read. I'm not familiar enough with yapf to know exactly which rule controls this. Maybe SPLIT_BEFORE_EXPRESSION_AFTER_OPENING_PAREN or SPLIT_BEFORE_FIRST_ARGUMENT?
|
Passing linter run: https://github.com/tensorflow/model-card-toolkit/actions/runs/4520740897 |
What does this pull request do?
This PR set up a basic linter configuration for the MCT repo. It is a replacement for the closed PR: #242
The
pylintrcis mimic'ing Google'spylintrc: https://github.com/google/styleguide/blob/gh-pages/pylintrcwith two additional disabled checks (due to quantity):
All other initial issues have been addressed to make the code base compliant with the
pylintrc:Fixes #246
How did you test this change?
No functionality of tests changes - only formatting and styling of the code itself.
How did you document this change?
CONTRIBUTING.mdhas been updatedBefore submitting
Before submitting a pull request, please be sure to do the following: