Openlibrary: Remove unused imports from files.

Created on 1 Jun 2020  路  20Comments  路  Source: internetarchive/openlibrary


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

Stakeholders


@cclauss @imskr

Good First Issue @mekarpeles 3 RefactoClean-up hacktoberfest

All 20 comments

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

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

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!

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.

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.

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

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

Can I too work on this issue, sir?

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

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?

@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.

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.

@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?

@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?

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

I thought that #3909 was that PR.

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.

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.

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

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

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.

Was this page helpful?
0 / 5 - 0 ratings