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

Fix pylint #522

Merged
merged 7 commits into from Nov 2, 2020
Merged

Fix pylint #522

merged 7 commits into from Nov 2, 2020

Conversation

nikita-klsh
Copy link
Member

This PR fixes pylint output in the latest docker image analysiscenter1/ds-py3.

A lot of missing-function-docstring and abstract-method warnings happened in batchflow/models/torch folder we cant fix, because they occur on the torch side. They are fixing this and this.

So we can:

  1. Close eyes on this warnings.
  2. Temporary set additional restrictions for pylint for torch folder only like its currently done.

@roman-kh

@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

Merging #522 into master will decrease coverage by 0.08%.
The diff coverage is 25.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #522      +/-   ##
==========================================
- Coverage   41.46%   41.38%   -0.09%     
==========================================
  Files         147      148       +1     
  Lines       14587    14843     +256     
==========================================
+ Hits         6049     6143      +94     
- Misses       8538     8700     +162     
Impacted Files Coverage Δ
batchflow/batch.py 55.53% <0.00%> (ø)
batchflow/dataset.py 88.54% <ø> (ø)
batchflow/decorators.py 27.73% <0.00%> (ø)
batchflow/dsindex.py 87.55% <ø> (ø)
batchflow/models/tf/fcn.py 27.45% <0.00%> (ø)
batchflow/models/tf/inception_base.py 15.51% <0.00%> (ø)
batchflow/models/tf/inception_resnet_v2.py 18.01% <0.00%> (ø)
batchflow/models/tf/inception_v1.py 35.71% <0.00%> (ø)
batchflow/models/tf/inception_v3.py 22.22% <0.00%> (ø)
batchflow/models/tf/inception_v4.py 19.27% <0.00%> (ø)
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8957da...da0a17a. Read the comment docs.

@nikita-klsh nikita-klsh linked an issue Sep 21, 2020 that may be closed by this pull request
@roman-kh
Copy link
Member

@nikita-klsh It seems pytorch issues are solved already.

@nikita-klsh
Copy link
Member Author

@nikita-klsh It seems pytorch issues are solved already.

@roman-kh There are no releases with updates yet. Or we want to build it from github?

@nikita-klsh
Copy link
Member Author

Running pylint with torch 1.7.0 still raises warnings about forward methods missing docs.
Torch solved the issue and now the method formally obtains the doc, i.e. nn.Module.forward.__doc__ is not None, even tho docs are not explicitly speсified in the method's body.
I suppose this might be pylint false warning.

@@ -75,7 +75,6 @@ def __init__(self, index, batch_class=Batch, *args, preloaded=None, cast_to_arra
self._attrs = None
kwargs['_copy'] = kwargs.get('_copy', copy)
self.n_splits = None
self.train, self.test = None, None
Copy link
Member

Choose a reason for hiding this comment

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

Why do you think this is a better approach?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks weird to reinitialize attributes, that have already been initialized in the parent class Baseset

Copy link
Member Author

Choose a reason for hiding this comment

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

I return it as it was initially and plug pylint: disable=attribute-defined-outside-init stub.
In the code we assign values to self.train / self.test / self.validation attributes that WERE defined in the init.
It looks like pylint false positive

@roman-kh roman-kh merged commit 9713b11 into master Nov 2, 2020
@roman-kh roman-kh deleted the fix_pylint branch November 2, 2020 08:34
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.

Fix pylint errors and warnings
2 participants