search

hammerlab / flowdec

TensorFlow Deconvolution for Microscopy Data
Apache License 2.0
86 stars 26 forks source link

Add linter and travis build #5

Closed eric-czech closed 5 years ago

russellb commented 5 years ago

I'll poke at this next.

russellb commented 5 years ago

See the top commits on https://github.com/russellb/flowdec/commits/master for one approach for this.

The first commit creates a tox (https://tox.readthedocs.io/en/latest/) configuration that runs the flake8 utility against the Python source. It disables all warnings necessary to make it pass successfully. My earlier patch dropping unused imports resolved one warning pointed out by flake8. Most of the remaining ones are related to Python formatting conventions (PEP 8). flake8 can also help find some real issues, like Python 3 compatibility things.

The second commit adds the travis-ci config, which just runs tox.

You can see an example of this config running on this test PR: https://github.com/russellb/flowdec/pull/4

russellb commented 5 years ago

I'll send it over as a PR if you're happy with it.

eric-czech commented 5 years ago

Looks good to me! The couple times I've gone through that before included adding the environments and running tests manually as part of the travis config so thanks for pointing out tox. I'll try to use that instead from now on.

eric-czech commented 5 years ago

Looks like this is all set now per https://github.com/hammerlab/flowdec/pull/7 .

Although, what are your thoughts on how to get code analysis as well style enforcement? I always appreciated the unused imports, misspelled variable names, unreachable code, etc. that comes from pylint so is there something better these days for that? Is that what you meant by other things flake8 can find?

russellb commented 5 years ago

Yeah, flake8 reports stuff like that. I've used flake8 more personally so that's what i set up here first. It seems there's some overlap, and some things each do that the other doesn't. More complete coverage would come from just running both, which shouldn't be too hard if you wanted to add it. It'd just be some additions to tox.ini, following the flake8 example.

On Wed, Oct 17, 2018 at 6:39 PM Eric Czech notifications@github.com wrote:

Looks like this is all set now per #7 https://github.com/hammerlab/flowdec/pull/7 .

Although, what are your thoughts on how to get code analysis as well style enforcement? I always appreciated the unused imports, misspelled variable names, unreachable code, etc. that comes from pylint so is there something better these days for that? Is that what you meant by other things flake8 can find?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hammerlab/flowdec/issues/5#issuecomment-430814129, or mute the thread https://github.com/notifications/unsubscribe-auth/AAS4CrcF1VwyGPTsmr62_H_evwpnS9JKks5ul7G9gaJpZM4XLXZ1 .

-- Russell Bryant

eric-czech commented 5 years ago

Definitely done now after adding unit tests to tox.ini (https://github.com/hammerlab/flowdec/commit/2421e3577432280b9e1db0c60f2ba6b8de5ccb3a)