improve partial Codecov

[Borda]

What does this PR do?

Partial work on #463 increasing to 91%

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[pep8speaks]

Hello @Borda! Thanks for updating this PR.

• In the file pytorch_lightning/__init__.py:

Line 23:101: E501 line too long (106 > 100 characters)
Line 24:101: E501 line too long (108 > 100 characters)

• In the file pytorch_lightning/callbacks/__init__.py:

Line 3:101: E501 line too long (101 > 100 characters)

• In the file pytorch_lightning/loggers/comet.py:

Line 23:101: E501 line too long (105 > 100 characters)

• In the file pytorch_lightning/loggers/mlflow.py:

Line 34:101: E501 line too long (103 > 100 characters)

• In the file pytorch_lightning/loggers/neptune.py:

Line 16:101: E501 line too long (104 > 100 characters)

• In the file pytorch_lightning/loggers/test_tube.py:

Line 7:101: E501 line too long (106 > 100 characters)

• In the file pytorch_lightning/loggers/trains.py:

Line 38:101: E501 line too long (103 > 100 characters)
Line 69:101: E501 line too long (107 > 100 characters)

• In the file pytorch_lightning/loggers/wandb.py:

Line 18:101: E501 line too long (102 > 100 characters)

Comment last updated at 2020-03-18 20:36:26 UTC

[codecov]

Codecov Report

Merging #1172 into master will increase coverage by 2%.
The diff coverage is 95%.

@@          Coverage Diff           @@
##           master   #1172   +/-   ##
======================================
+ Coverage      89%     91%   +2%     
======================================
  Files          62      62           
  Lines        3164    3093   -71     
======================================
- Hits         2810    2802    -8     
+ Misses        354     291   -63     

[williamFalcon]

@Borda no tensorboardx... keep it pytorch summaryWriter please

[Borda]

@bmartinn could you help why the Trains tests are failing?

[bmartinn]

@Borda sure, I'll take a look.
I think I have an idea on what the problem is, I'll work on a fix.

[bmartinn]

@Borda I just pushed a fix as a PR on the Codecov branch
See original commit here (based on codecov branch)
bmartinn@8b99bcb

[Borda]

@bmartinn merger you PR here but the failer is the same?
is seems that it complains about existing names or so?
Also curious, does it work also offline?

[bmartinn]

Hi @Borda,

It seems like two Loggers are running simultaneously, is this possible ?

Trains is all about logging all the outputs/logs/console/artifacts to the trains-server (usually self hosted). So yes, currently there is no offline mode. That said I could easily add "bypass" argument, making sure the interface works, and it will just bypass sending data to the trains-server.

What do you think?

[Borda]

@bmartinn Well we have not changed anything on CI side and it was passing before..
In particular, we are sure that Drone runs only one ob in concurrency, @luiscape ring?

[bmartinn]

Hmmm, anyhow it might be safer to have "bypass_mode"...
@Borda, I can PR a commit on the Codecov branch if you like

[Borda]

@bmartinn that would be great! Thx

[bmartinn]

@Borda , Done :)
#1187

[williamFalcon]

awesome! let's keep getting the coverage up :)

[neggert]

Looks good, but at some point we should do an audit of lines that have pragma: no-cover. IMO some of theses should be included in coverage.

[Borda]

Looks good, but at some point we should do an audit of lines that have pragma: no-cover. IMO some of theses should be included in coverage.

I do agree, we need to work more on tests, but with this kind of commented lines and functional visual codecov UI it is much easier to spot weakness in the package...
Note that this is just one of several upcoming PRs on getting better coverage but I would like to keep it in smaller chunks :]