Investigate Python Unused Imports

[teymour-aldridge]

There are a lot of unused imports which clutter things.

Describe the problem that you'd like solved

Refactoring to remove all unused imports.

Proposal & Constraints

Remove all ununsed imports. This will reduce technical debt, decluttering things to make them clearer.

Additional context

UPDATE:
As per @cclauss's comments below, for detecting unused imports, we should really be running

python3 -m pip install flake8
flake8 --select=F401

Stakeholders

@cclauss @imskr

[mekarpeles]

@teymour-aldridge could you help us with this one?

[imskr]

Work is in Progress refer #3447 As soon as this will get merged, I will land another PR.

[teymour-aldridge]

I'd be happy to help. I'm a bit busy at the moment, but should have some time on the weekend.
To automatically catch these sort of errors it might be worth installing CodeClimate on the repository (which automatically reports code duplication, ununsed imports, overly complex functions, etc) as well as LGTM.com (which catches security bugs in open source). Both are free for open sorce software!

[cclauss]

flake8 which we already run in our continuous integration can autodetect these issues.

Detection is not the problem. The problem is that no one is reviewing PRs so they sit open for months at a time. Let's not add CodeClimate as it is full of uncontrollable false positives.

[teymour-aldridge]

Oh right. I may have jumped the gun a bit here. I'd be happy to work to try and help resolve some of these issues.

[ctorok]

Happy to help; need some guidance. These are the steps I would take?

1. Clone the repository at https://github.com/internetarchive/openlibrary
2. Run the query and get the results, https://lgtm.com/query/7596990423930761565/
3. Remove offending import from file
4. Commit back to repo

[cclauss]

python3 -m pip install flake8
flake8 --select=F401

[Himanshunitrr]

Can I too work on this issue, sir?

[cclauss]

Please refer to #3447... That must be reviewed and landed first.

[SaravgiYash]

Hi, @cclauss @imskr I would like to work on this issue.

Can I remove unused imports from other files excluding code.py and conftest.py?

[SaravgiYash]

@mekarpeles I started working on this issue, so I just want to know that is there a way to automatically remove unused imports or do I need to remove them manually.

[cclauss]

CAUTION: Some imports are done for side effects so linters like flake8 may say they are not needed but things will break without them. Please do just a file or two per PR.

[SaravgiYash]

@cclauss I have removed unused imports till .\openlibrary\plugins\openlibrary\utils.py so should I create a PR and commit them 2 at a time? or create separate PR for each of them?

[SaravgiYash]

@cclauss I would like to remove unused imports from other files as well. So should I first create a PR for unused Python standard library imports?

[cclauss]

Should I first create a PR for unused Python standard library imports?

I thought that #3909 was that PR.

[SaravgiYash]

I thought that #3909 was that PR.

@cclauss I mean for other files (files after openlibrary/core/waitinglist.py).
I removed all unused imports till openlibrary/core/waitinglist.py and in PR #3909 I removed all standard unused imports till openlibrary/core/waitinglist.py.

[cclauss]

OK. Please creating more PRs that only remove Python standard library imports. Once those are all landed, we can look at PRs for other imports.

[SaravgiYash]

@cclauss As #3965 is merged should I remove other unused imports and commit them 2 files at a time?

[SaravgiYash]

@cclauss Should I go ahead and remove other unused imports (Non-standard ones)

[cclauss]

Sorry but not right now. These changes create issues that are quite difficult to find.

Right now we are trying to upgrade to Python 3 and need to maximize stability to deal with bytes vs. str issues.

[Atheriz-46]

Hey, is this issue still open?? I can take it up.

[cclauss]

It is still open but as mentioned above, we have a fair number of imports that have side-effects. This means that flake8 F401 might say that an import is not needed but removing it will create a crash/bug/etc. We are super-close to removing Python 2 and once that is done, we can go full force on this issue.

If you want to dig in now, just modify a single file per pull request. If PRs land cleanly, that is great. If they do not land cleanly, we can add comments that describe the side effects. Thanks for your interest!

[Enigma04]

if this issue is still open I would like to take it up

[cclauss]

The issue is open but we have not removed the Python 2 code so let's hold off on attacking this one a little bit longer.

[vatsal-cmd]

I want to be a participant in solving this issue, i am a beginner can anyone tell me from where to start and till now what is the progress for this issue?

[cclauss]

let's hold off on attacking this one a little bit longer.

[soham4abc]

Can anyone please have a look at my PR It has been open for a while with no reviews! @cclauss @cdrini

[ItsApex]

Hey @cclauss Is this issue still open I would like to contribute

[Eds-Dbug]

@cclauss, I would like to be next in line to take up this issue.

[cclauss]

The comments above still apply:

It is still open but as mentioned above, we have a fair number of imports that have side effects. This means that #3483 (comment) might say that an import is not needed but removing it will create a crash/bug/etc. If you want to dig in, modify one or two files per pull request. If PRs land cleanly, that is great. If they do not land cleanly, we can add comments that describe the side effects. Thanks for your interest!