search

markovmodel / PyEMMA

🚂 Python API for Emma's Markov Model Algorithms 🚂
http://pyemma.org
GNU Lesser General Public License v3.0
307 stars 119 forks source link

Do automatic code checking for entire package before release. #202

Closed franknoe closed 9 years ago

franknoe commented 9 years ago

My PyCharm still reports syntax errors in the current dev_release_1.2 branch. Common example: trying to pass the wrong number of arguments to a function, and this case is not covered by unit-tests.

Use PyCharm or another semi-intelligent tool to check all code before the release.

marscher commented 9 years ago

maybe we should write a pylint ruleset and add it to travis. Does PyCharm perform more than static code analysis?

franknoe commented 9 years ago

It finds dynamic code usages too but doesn' evaluate them, so just gives hints.

I'm all for an automatic solution in Travis, but in my opinion it doesn't free the programmer from doing checks on their own code before submitting a PR. So we should aim at standartizing this process such that unnecessary bugs that occur now are avoided in the future as early as possible in the process. Saves time and nerves for everyone.

Am 08/04/15 um 15:10 schrieb Martin K. Scherer:

maybe we should write a pylint ruleset and add it to travis. Does PyCharm perform more than static code analysis?

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/PyEMMA/issues/202#issuecomment-90910310.

Prof. Dr. Frank Noe Head of Computational Molecular Biology group Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354 Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

marscher commented 9 years ago

I've added pylint to travis on my branch:

https://travis-ci.org/marscher/PyEMMA/builds/57649196

It first runs the test-suite and then analyses the code and prints all found errors. This is an initial hint for pull requesters.

If you are happy with this, I'll merge it now.

marscher commented 9 years ago

I can also make this check fatal, so in case the return value of pylint is not zero, the travis build will fail.

franknoe commented 9 years ago

Many of these error reports appear not to be real (e.g. an object has no shape... is this really an error or does the code-checker not know that it's an array?). I would merge only if we can get clean bug reports, otherwise we have to go through all bug reports manually to see which ones are real and need fixing.

Priority now is getting all new coordinate stuff working an tested - pleasy aim at having this done by friday. I'm happy to run a codecheck through my PyCharm on the weekend to point out minor issues.

Am 08/04/15 um 17:41 schrieb Martin K. Scherer:

I've added pylint to travis on my branch:

https://travis-ci.org/marscher/PyEMMA/builds/57649196

It first runs the test-suite and then analyses the code and prints all found errors. This is an initial hint for pull requesters.

If you are happy with this, I'll merge it now.

— Reply to this email directly or view it on GitHub https://github.com/markovmodel/PyEMMA/issues/202#issuecomment-90955154.

Prof. Dr. Frank Noe Head of Computational Molecular Biology group Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354 Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany