Testing for unused imports is possible (pyflakes does it), but I am -1 to adding an automated test for it. Unused imports are clutter, and PRs removing them are welcome, but they are not so bad that they need to be always clean at every point in time. Also there can be false positives, because a function can be imported just so that it can be recursively imported from that same module. At best, you have to ignore __init__.py files.
Aaron Meurer On Wed, Jan 7, 2015 at 9:32 AM, Joachim Durchholz <[email protected]> wrote: > Am 07.01.2015 um 17:08 schrieb Sergey Kirpichev: > >> On Wed, Jan 7, 2015 at 6:31 PM, Joachim Durchholz <[email protected]> >> wrote: >> >>> I'm not sure that that is possible. >>> >> >> That's bad news. >> >> With any other order, the next person to git pull will get a nasty >>> surprise: >>> all their bin/test runs will fail with those newly tested-for errors, so >>> they can't rely on that "I can make the PR as soon as all tests pass" >>> rule >>> anymore. >>> >> >> You can break other people PR's in a zillons of ways. For example, >> their pr can rely on removed import. >> > > Yes, but then they have to merge anyway. > If I add a new test that goes over all source files, they will have not > only their changes flagged but also pre-existing code. > > However, I'd also like to know if such fixes are acceptable if it turns >>> out >>> we can't make it an automated test. >>> >> >> Sure, they are. But at least, please try to minimize number of such PR's. >> (I.e., do this for all modules once) >> > > That's going to be a huge PR that touches all of SymPy. > Remember: 1,000+ reports of unnecessary imports (including false > positives, but I bet it's not going to be much less than 900 changed lines). > > That's going to be hard on the eyeball checking. > Of course people might find it worth it anyway. It's a merely technical > change, so as long as the tests pass, it *should* be fine. > > It's also going to be more work to keep the PR mergeable as other PRs go > in. > Though... I could do that *shrug*. > > Let me see what others say. If it's okay by at least one additional > contributor, I'll go ahead and do a big PR for that. > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/sympy. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/sympy/54AD5FAB.2090604%40durchholz.org. > > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sympy. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAKgW%3D6LbzX6j%3D%2Bfd%2BpEvqAk%2Bw_vJvx6jmobaviDs9bm1mqVoMA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
