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

Remove unused imports #121

Merged
merged 1 commit into from
Sep 13, 2020
Merged

Conversation

hakonanes
Copy link
Member

@hakonanes hakonanes commented Sep 11, 2020

Signed-off-by: Håkon Wiik Ånes hwaanes@gmail.com


name: Pull request
about: Remove unused imports


What does this PR do? Please describe and/or link to an open issue.
All unused imports are removed. All tests pass locally.

Describe alternatives you've considered
There are also some places in the code where functions have parameters they don't use (e.g. https://github.com/pyxem/diffsims/blob/master/diffsims/generators/diffraction_generator.py#L200), and lines of code irrelevant for the output (https://github.com/pyxem/diffsims/blob/master/diffsims/libraries/diffraction_library.py#L140). This is not me being extremely attentive, it's just PyCharm showing unused variables with some transparency.

Should I remove this in this PR?

Work in progress?

  • Remove unused function parameters and code bits

Signed-off-by: Håkon Wiik Ånes <hwaanes@gmail.com>
@hakonanes hakonanes added the dev label Sep 11, 2020
@hakonanes hakonanes added this to the v0.3.0 milestone Sep 11, 2020
@hakonanes hakonanes self-assigned this Sep 11, 2020
@hakonanes hakonanes marked this pull request as draft September 11, 2020 15:59
@pc494
Copy link
Member

pc494 commented Sep 11, 2020

This is good & was on my list of things to do at some point. What did you use to automate this?

@hakonanes
Copy link
Member Author

This is good & was on my list of things to do at some point. What did you use to automate this?

Went through all the files by hand...

@pc494
Copy link
Member

pc494 commented Sep 11, 2020

ah. We can add "look up a way to automate this" for the future, for now you dedraft this and I'll merge it.

@hakonanes
Copy link
Member Author

Could you just quickly tell me if I should remove the unused bits mentioned in part 2 of my top comment?

@pc494
Copy link
Member

pc494 commented Sep 11, 2020

sorry, was being slow and hadn't clocked pyCharm caught more than just imports!

  1. leave the magnitude_tolerance, I'll catch it in the sweep in Sweeping for release: ProfileSimulation and updated dependancies #119
  2. you can leave the other one too, I'll deal with it in the sweep as the code is a bit more wrong than the highlighting suggests

summary: this is good as is, will use #119 to address the leftovers

@hakonanes hakonanes marked this pull request as ready for review September 11, 2020 18:18
@tjof2
Copy link

tjof2 commented Sep 12, 2020

flake8 and a similar config to that in pyxem would make it simpler to automatically identify things like this. F401 is an unused import. You can even make it part of the tests with pytest-flake8...

https://github.com/pyxem/pyxem/blob/3e9662c7a78c488e6d1844986bb53d0eb4fbbb8d/setup.cfg#L23

@pc494 pc494 merged commit 200c26f into pyxem:master Sep 13, 2020
@hakonanes hakonanes deleted the remove-unused-imports branch September 13, 2020 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants