search

ocean-data-factory-sweden / kso

Notebooks to upload/download marine footage, connect to a citizen science project, train machine learning models and publish marine biological observations.
GNU General Public License v3.0
5 stars 12 forks source link

Detect unused code #282

Closed Diewertje11 closed 1 year ago

Diewertje11 commented 1 year ago

This commits implements a new part of the CI where we use Vulture to detect unused code. This check fails if it finds un-used code with a confidence of 60% or higher.

The whitelist.py is a file in which we can state all errors Vulture reports that we have checked and want to keep. Vulture states many false positives and therefore this is needed.

So the idea is to check everything and put all false positives in the whitelist. Once in the so much time we should clear the whitelist and recheck everything.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Diewertje11 commented 1 year ago

This is working now as well! The unused-code check is failing since we have a lot of unused code. We can decide to merge it now into dev and then follow up in a new PR to fix all the unused code, or do that immediately in this MR.

Diewertje11 commented 1 year ago

Only rebased on top of the new dev

Diewertje11 commented 1 year ago

Looks good to me. I'm just wondering if the adding of print statements to each notebook should be in this PR as well? Or maybe just a separate one? We can leave it in as well it's not major.

Without the addition of the print statement to each notebook, the jupytext cannot handle it and the whole test does not work. So it is needed in this PR.

jannesgg commented 1 year ago

Looks good to me. I'm just wondering if the adding of print statements to each notebook should be in this PR as well? Or maybe just a separate one? We can leave it in as well it's not major.

Without the addition of the print statement to each notebook, the jupytext cannot handle it and the whole test does not work. So it is needed in this PR.

Great! Now I understand.