style: fix E501, F401, E711 on mask, saveresults, selfcalibrate, and srxplanar.py #16
Conversation
src/diffpy/srxplanar/mask.py
Outdated
| try: | ||
| rv = tifffile.imread(im) | ||
| except: | ||
| except ImportError: |
There was a problem hiding this comment.
To me, it seems like we are assuming that tifffile is imported properly, and that we are calling a function from that package inside the try block. So maybe OSError or ValueError would make sense here? Because we want to make sure any image loading errors are handled properly?
There was a problem hiding this comment.
Updated in new commits.
| rcParams["backend"] = "Qt4Agg" | ||
| try: | ||
| import PySide | ||
| import PySide # noqa: F401 |
There was a problem hiding this comment.
Because flake8 gives F401 error here, but actually I think it is implicitly used in the next line rcParams["backend.qt4"] = "PySide". Therefore, we should not delete it, that's why I make #noqa here.
| try: | ||
| import matplotlib.pyplot as plt | ||
| import PyQt4 | ||
| import PyQt4 # noqa: F401 |
There was a problem hiding this comment.
This import seems safe to delete. By the way, I'm kind of nitpicking here because in general, we don't want to suppress any warnings or exclude any files if possible
There was a problem hiding this comment.
Have deleted it, see the new commit.
src/diffpy/srxplanar/mask.py
Outdated
| try: | ||
| rv = tifffile.imread(im) | ||
| except: | ||
| except ValueError: |
There was a problem hiding this comment.
Actually, maybe include OSError as well? Should have mentioned that, sorry
There was a problem hiding this comment.
Have updated in the new commit.
|
@sbillinge Ready to review this one |
3 participants
@zmx27 ready to review