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

use pyflakes to identify simple bugs in sympy and fix them #4555

Closed
certik opened this issue Jun 4, 2009 · 19 comments
Closed

use pyflakes to identify simple bugs in sympy and fix them #4555

certik opened this issue Jun 4, 2009 · 19 comments
Labels
Bug Code quality Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. imported

Comments

@certik
Copy link
Member

certik commented Jun 4, 2009

Example:

sudo apt-get install pyflakes

$ pyflakes sympy/integrals/
sympy/integrals/rationaltools.py:3: 'div' imported but unused
sympy/integrals/rationaltools.py:121: redefinition of unused 'symbols' from
line 3
sympy/integrals/risch.py:4: 'Pow' imported but unused
sympy/integrals/risch.py:5: 'Function' imported but unused
sympy/integrals/risch.py:7: 'Atom' imported but unused
sympy/integrals/risch.py:8: 'Integer' imported but unused
sympy/integrals/deltafunctions.py:2: 'Symbol' imported but unused
sympy/integrals/deltafunctions.py:2: 'S' imported but unused
sympy/integrals/deltafunctions.py:2: 'Wild' imported but unused
sympy/integrals/integrals.py:2: 'Pow' imported but unused
sympy/integrals/integrals.py:9: 'apart' imported but unused
sympy/integrals/integrals.py:10: 'limit' imported but unused
sympy/integrals/integrals.py:13: 'DiracDelta' imported but unused
sympy/integrals/integrals.py:13: 'Heaviside' imported but unused
sympy/integrals/integrals.py:111: redefinition of unused 'limit' from line 10
sympy/integrals/__init__.py:8: 'integrate' imported but unused
sympy/integrals/__init__.py:8: 'line_integrate' imported but unused
sympy/integrals/__init__.py:8: 'Integral' imported but unused
sympy/integrals/tests/test_rationaltools.py:4: 'log_to_atan' imported but
unused
sympy/integrals/tests/test_rationaltools.py:4: 'log_to_real' imported but
unused
sympy/integrals/tests/test_rationaltools.py:4: 'ratint_ratpart' imported
but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'cos' imported but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'Integral' imported but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'sympify' imported but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'integrate' imported but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'diff' imported but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'pi' imported but unused
sympy/integrals/tests/test_lineintegrals.py:1: 'sin' imported but unused
sympy/integrals/tests/test_integrals.py:1: redefinition of unused 'atan'
from line 1
sympy/integrals/tests/test_integrals.py:1: 'I' imported but unused
sympy/integrals/tests/test_integrals.py:5: 'skip' imported but unused
sympy/integrals/tests/test_integrals.py:5: 'XFAIL' imported but unused


it finds (among other things) that the Heaviside is imported but never
used, so it should be removed.

Original issue for #4555: http://code.google.com/p/sympy/issues/detail?id=1456
Original author: https://code.google.com/u/104039945248245758823/

@rlamy
Copy link
Member

rlamy commented Dec 8, 2009

See http://groups.google.com/group/sympy/browse_thread/thread/d073a9ad2d19845 Priit wrote a tool to get a useful pyflakes report. It's at
git://github.com/plaes/sympy.git branch 'pyflakes'. 
I've pulled from it and added a commit removing some unused imports in
sympy/functions/ in my github repo (git://github.com/rlamy/sympy.git, 'pyflakes').

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c1
Original author: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

asmeurer commented Dec 8, 2009

There is also pychecker, which shows a lot of things similar to this (I can't get pyflakes to work, so I can't say if it 
does more or not).  You can just install and run pychecker sympy --limit 1000 to see them all (it takes a few 
minutes to run).  I've attached a file of my output.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c2
Original author: https://code.google.com/u/asmeurer@gmail.com/

@c00w
Copy link

c00w commented Mar 23, 2011

I was going to make a patch but I got really lazy. here is my pyflakes output with some of the correct but misidentified things removed.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c3
Original author: https://code.google.com/u/103714092292933486699/

@Krastanov
Copy link
Member

PyLint seems like a better choice according to http://stackoverflow.com/questions/1428872/pylint-pychecker-or-apyflakes

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c4
Original author: https://code.google.com/u/100157245271348669141/

@asmeurer
Copy link
Member

If I remember correctly, pylint can be customized, right?  Because I remember that it complained about a bunch of stuff that (in my opinion anyway) was not too important (like lines only slightly longer than 80 chars) , or should actually remain the same (like no spaces around *).

By the way, except for the fact that it's much slower, the fact that pychecker runs the code isn't that big of a deal for sympy.  There will be a handful of things that tools like pylint will incorrectly warn about (because they seem to be wrong in a static sense), but (assumedly) pychecker won't.

Maybe we should just edit the ./setup.py audit command so it can use any of the three.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c5
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Labels:** CodeInCategory-QA CodeInDifficulty-Medium  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c6
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Labels:** CodeInImportedIntoSpreadsheet  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c7
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

A note: if you find an actual bug with this (rather than just some code quality problem), a test should be added for it.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c8
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Labels:** CodeInMultiple  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c9
Original author: https://code.google.com/u/asmeurer@gmail.com/

@rlamy
Copy link
Member

rlamy commented Nov 16, 2011

Note that there are lots of warnings coming from polys/densepolys.py, polys/densetools.py and polys/sparsepolys.py, but these modules are known not to work, cf. issue 5470 .

**Blockedon:** 5470  

Referenced issues: #5470
Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c10
Original author: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

**Labels:** -codeinimportedintospreadsheet CodeInImportedIntoMelange  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c11
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

So pyflakes doesn't really warn about many things that shouldn't be fixed, but pylint does.  So if you use pylint, don't bother with things like too short or too long variable names or any "too many..." warnings.  

And of course, if pyflakes says something that turns out to be wrong, don't fix it.  This is just the nature of dynamic languages like Python, that you can not always tell 100% what is happening just by looking at the code.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c12
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Status:** Valid  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c13
Original author: https://code.google.com/u/asmeurer@gmail.com/

@jrioux
Copy link
Member

jrioux commented Sep 25, 2012

There was work on this at https://github.com/sympy/sympy/pull/738

**Blockedon:** -sympy:2371 sympy:2371  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c14
Original author: https://code.google.com/u/102137482174297837682/

@smichr
Copy link
Member

smichr commented Oct 19, 2012

There are still many undefined variables which is bad: this means that an error is waiting to happen in code that is not being tested.

e.g.

sympy/benchmarks/bench_meijerint.py:7: W802 undefined name 'inverse_fourier_transform'
sympy/benchmarks/bench_meijerint.py:8: W802 undefined name 'inverse_laplace_transform'
sympy/benchmarks/bench_meijerint.py:9: W802 undefined name 'inverse_mellin_transform'
sympy/categories/diagram_drawing.py:2082: W802 undefined name 'obj'
sympy/core/tests/test_function.py:231: W802 undefined name 'oo'

And there is a lot of cruft in terms of unused variables which is less critical but could be a good place for beginners to start.

e.g.


sympy/abc.py:1: W402 'Symbol' imported but unused
    it's in an exec

sympy/galgebra/GA.py:2249: W806 redefinition of function 'div' from line 2238
sympy/galgebra/tests/test_GA.py:83: W806 local variable 'Bsq' is assigned to but never used
sympy/galgebra/tests/test_GA.py:116: W806 local variable 'e' is assigned to but never used

I don't know if this issue will ever close. These things crop up like weeds unless the pep8/pyflakes/flake8 tools are not used regularly.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c15
Original author: https://code.google.com/u/117933771799683895267/

@asmeurer
Copy link
Member

**Labels:** -CodeInImportedIntoMelange  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c16
Original author: https://code.google.com/u/asmeurer@gmail.com/

@Krastanov
Copy link
Member

**Labels:** CodeInImportedIntoGoogleDocs  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c17
Original author: https://code.google.com/u/100157245271348669141/

@asmeurer
Copy link
Member

asmeurer commented Nov 8, 2012

**Labels:** -CodeInDifficulty-Medium  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1456#c18
Original author: https://code.google.com/u/asmeurer@gmail.com/

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
@oscarbenjamin
Copy link
Collaborator

Since we now use flake8 on Travis and the codebase is free of flake8 warnings I'll close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Code quality Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. imported
Projects
None yet
Development

No branches or pull requests

10 participants