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

coding style #4856

Open
sympy-issue-migrator opened this Issue Dec 10, 2009 · 47 comments

Comments

Projects
None yet
8 participants
@sympy-issue-migrator
Copy link

sympy-issue-migrator commented Dec 10, 2009

The file fem.py presents incorrect identation.  Partially with 3, partially
with 7 spaces.

Maybe it is not a big issue, but for pyDev users like me, it issues a lot
of warnings which hides other information.

I will take care of this issue myself.

As I am a newbie, can someone explain me how I submitt a patch, where and
to whom?

Thanks in advance,

idichekop

Original issue for #4856: http://code.google.com/p/sympy/issues/detail?id=1757
Original author: https://code.google.com/u/idichekop@gmail.com/

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 9, 2009

Thanks for finding and offering to fix this.  Are there any other files that have indentation problems?  We have a 
code_quality test, but it doesn't seem to test for that (we need to change this).  

It you are familiar with git, that is what we use.  If you are not, here are some tutorials: http://code.google.com/p/sympy/wiki/GitTutorials .

**Status:** Accepted  

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 9, 2009

And by the way, just post the patch file to this issue.

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 9, 2009

**Summary:** fem.py with incorrect identation  

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

@sympy-issue-migrator

This comment has been minimized.

Copy link

sympy-issue-migrator commented Dec 10, 2009

So here is the patch with the correct indentation.

I noticed that several other files have the same problem.  May I correct them?
It goes fast... I have a script for that :D! 

In positive case, may I modify everything recursively and post only one patch?

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c4
Original author: https://code.google.com/u/idichekop@gmail.com/

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 10, 2009

Yes, please go for it, this would be great. Just make sure that the tests still pass 
and the examples still work (2 of them are broken anyway AFAIK, so don't worry if these 
fail after your changes). If you want, you could even improve our code quality tests to 
check for correct indentation.

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 10, 2009

Maybe you could also implement your script in bin/strip_whitespace.

**Labels:** Milestone-Release0.6.6  

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

@sympy-issue-migrator

This comment has been minimized.

Copy link

sympy-issue-migrator commented Dec 11, 2009

What does strip_whitspace do? 
The script I used already cuts out all extra white spaces at the end of the line :D

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 11, 2009

This, and it converts line endings to the unix way.

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

@sympy-issue-migrator

This comment has been minimized.

Copy link

sympy-issue-migrator commented Dec 11, 2009

Ok.  Here comes the patch with ALL the indentation corrections that I could find.

Note that a lot of files were altered. After all the modifications I ran the tests
/bin/test and everything was ok (with exception of some hilbert dingsbumbsda whose
testbench was not correct ).

Can anyone check these tests again before committing the patch to the repository? It
is the first time I ran the tests myself, and I am not sure if I covered everything
right.

Anyway.  The code now looks much better :D! I hope it helps.

Best, Julio

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 11, 2009

Wow, that is a lot of changes.  Don't worry, it is standard practice to run the tests before committing.

All tests pass for me.  What is this hilbert thing you are referencing?  

I noticed some changes like this:

@@ -936,29 +936,29 @@ def checkodesol(ode, func, sol, order='auto', solve_for_func=True):
         order = ode_order(ode, func)
     if solve_for_func and not (sol.lhs == func and not sol.rhs.has(func)) and not \
         (sol.rhs == func and not sol.lhs.has(func)):
-            try:
-                solved = solve(sol, func)
-                if solved == []:
-                    raise NotImplementedError
-            except NotImplementedError:
-                pass
+        try:
+            solved = solve(sol, func)
+            if solved == []:
+                raise NotImplementedError
+        except NotImplementedError:
+            pass
+        else:

I guess the change is fine, but I think that if you implement your script in strip_whitespace (which it would still be great if you 
did), then maybe you should leave that out.  Personally, I think it is easier to read the code when lines below nested if 
statements are indented one level deeper than the deepest part of the if statement, but the point is that I don't think we 
should force either way in this case in strip_whitespace.

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 11, 2009

@rlamy

This comment has been minimized.

Copy link
Member

rlamy commented Dec 12, 2009

Thanks, Julio! I've pushed your patch in (next time, please use line breaks in your
commit message). Now if you could add your script to the existing tools, it'd be great.

**Summary:** Tools to check and correct indentation  
**Status:** Started  
**Labels:** -Type-Defect -Milestone-Release0.6.6 -PassedReview Type-Enhancement Milestone-Release0.7.0  

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

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 14, 2009

It would probably be a good idea to include in the tests a test for hidden conditions
which won't show up in coverage, e.g. 

if: foo
else: bar

In the same spirit as this issue's patch and the one for commit 3083631... (for which
I couldn't find an issue) I have pushed to branch 'exposure' at smichr's github acct
a branch that opens all the closed if/else/elif statements and unindents all
(hopefully appropriate) places where a return is followed by an else-block. There
were also a HUGE number (1360 hits in 97 files) of trailing whitespaces in pyglet.
I've included fixes to those, but we should see if they could use some of the same
code quality tools we're using so we don't add code like that.

So again, branch `exposure` for review is at smichr's github. All test() and
doctest() pass here.

**Cc:** Vinzent.Steinberg asmeurer  
**Labels:** NeedsReview  

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 14, 2009

I am not sure how I feel about changing:

if test:
    return something
else:
    do something else

to

if test:
    return something
do something else

Coverage is not affected by this, and it can reduce readability in my opinion.  Is this mentioned in one of the code quality PEPs somewhere?

Also, I noticed that test_code_quality skips the third party directory, as well as __init__.py files, mpmath, and plotting.  Why is this?

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 14, 2009

I was referring to PEP 8, and it doesn't mention this.  Unless there is some other one that speaks about it, other 
than implicitly by PEP 20 :) (maybe actually "Flat is better than nested" applies here).  

By the way, has anyone here ever looked at PythonTidy?  It tries to change code to match PEP 8.  A lot of it I think 
we should not do, but some of the stuff could make the SymPy code much easier to read.

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

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 14, 2009

I suppose the reason for skipping third party and mpmath is that ideally we aren't
maintaining them...that should be handled upstream. But I think we should so easy
things like strip trailing whitespace and encourage them to do the same. Perhaps
__init__.py is skipped because it does things like "from foo import *". The runtests
blacklist has notes as to why certain things are skipped. The blacklist can be
over-ridden as I recall by putting in an explicit path to the thing you want tested.

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

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 14, 2009

Julio, you might compare your reindent output with the existing tool found at http://coverage.livinglogic.de/Tools/scripts/reindent.py.html

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 14, 2009

That's what I was thinking at first for __init__.py files, but back in issue 4553 , we were adding all manner of 
variables into the namespace because __init__.py in ntheory had from import * statements instead of explicit 
imports.  So we don't even want them there.

 Anyway, if I remove __init__.py from the blacklist, the test still passes, so I say we remove it and keep it out.  
Removing the other three cause the test to fail. There is also a comment that says that we need to fix mpmath 
and plotting.

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

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 15, 2009

regarding comment 14: I felt that way at first (about changing the indenation
structure) but I have come to prefer the if-return/do-more to the
if-return/else-do-more since when I see indentation and I start to work on one of the
indented logical blocks, I'm thinking about the different logical states that exist
at that point. If there is no indentation I more easily just put the previous state
out of mind.(*) Also, it's easier to keep lines within 80 chars if you don't keep
creeping to the right with extra indentation that is not actually needed.

(*) a tool I would like to see is one that prints code on the left and on the right
shows the logical context e.g.

C O D E     C O N T E X T
            all states
if a:       
  foo       a
else:       not a
  bar       "
  return    "
baz         all states
            all states
if a:       
  foo       a
  if b:
    bar     a and b
    return
else:       not a
  baz
baw         not (a and b)

Anyone seen such a thing?

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

@sympy-issue-migrator

This comment has been minimized.

Copy link

sympy-issue-migrator commented Dec 15, 2009

Hello all,

"
Comment 17  by smichr, Yesterday (20 hours ago)
Julio, you might compare your reindent output with the existing tool found at http://coverage.livinglogic.de/Tools/scripts/reindent.py.html "

Well, there must be a misunderstanding. I didn't say I MADE a script to correct the
indentation.  I said I HAD a script to correct the indentation. And that is exactly
this one you found :D!  Even if I got it from other source, they are identical :D!

Best,

Julio

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c20
Original author: https://code.google.com/u/idichekop@gmail.com/

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Dec 15, 2009

Ah.  Is that script open source so we can include it in SymPy then?

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 15, 2009

Chris, please rebase on master. Or do it rather after the release, if you want to
have this in for 0.7.

As usual, mpmath and pyglet should be fixed upstream. :) We should add documentation
about coding style, so we don't have to fix things like this in future. And I'm not
sure about removing the unnecessary else blocks. This should maybe discussed on the
list. However I'm +1 for converting one-line if-blocks.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c22
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 15, 2009

> Ah.  Is that script open source so we can include it in SymPy then?

From the reindent script:

# Released to the public domain, by Tim Peters, 03 October 2000.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c23
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 15, 2009

And it is already included in Python.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c24
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 15, 2009

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 15, 2009

I was following the lead of "Refactor trigonometric function code with no change in
behaviour"...figured if we liked this a little we might like it a lot.

The branch is rebased on master and again at `exposure`

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Dec 15, 2009

Looks fine to me. We need to add the coding conventions to our documentation.
And we should send patches to pyglet and mpmath upstream, changing sympy's copy is
pointless.

**Summary:** coding style  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c27
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 16, 2009

pyglet has been notified of all the changes in branch 1509pyglet. (Fixing whitespace
in pyglet is so easy and it's not pointless until the next release. Let's at least
fix that.)

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

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 16, 2009

I've also notified mpmath of the changes available at smichr's 1509mpmath branch at
github.

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Feb 3, 2010

I trivially rebased your branch on master, but I get the GA tests failing.

It seems that latex_ex.py is broken:

      return(name_str)

        #convert trailing digits to subscript
            m = regrep.match('(^[a-zA-Z]+)([0-9]+)$',name_str)
            if m is not None:
                name, sub=m.groups()
                tex=self._print_Symbol(Symbol(name))
                tex="%s_{%s}"

You'll get an unexpected indentation of course. Otherwise I'm fine with the changes.
Aaron, do you object something?

**Labels:** -NeedsReview NeedsBetterPatch  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c30
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Feb 3, 2010

Are we talking about 1509 or one of the mpmath/pyglet branches?  1509 could have one of the commits 
squashed into another, since it just fixes a whitespace error that the other introduces.

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Feb 3, 2010

This isn't holding anything, but I still think we should make the change I suggested in comment 18 above, namely, apply this:

diff --git a/sympy/utilities/tests/test_code_quality.py b/sympy/utilities/tests/test_code_quality.py
index e541a4a..379c4d4 100644
--- a/sympy/utilities/tests/test_code_quality.py
+++ b/sympy/utilities/tests/test_code_quality.py
@@ -95,7 +95,7 @@ def check_directory_tree_imports(p, exclude):

 def test_implicit_imports():
     """
-    Tests that all files except __init__.py use explicit imports,
+    Tests that all files, including __init__.py, use explicit imports,
     even in the docstrings.
     """
     path = split(abspath(__file__))[0]
@@ -104,7 +104,6 @@ def test_implicit_imports():
     examples_path = abspath(join(path + sep + pardir, "examples"))
     exclude = set([
         "%(sep)sthirdparty%(sep)s" % sepd,
-        "%(sep)s__init__.py" % sepd,
         # these two should be fixed:
         "%(sep)smpmath%(sep)s" % sepd,
         "%(sep)splotting%(sep)s" % sepd,

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Feb 4, 2010

And before closing this, we need to document our coding style, or we will have to fix
it constantly and keep telling contributors.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c33
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Jan 4, 2011

Here are some other issues that have come up and can be discussed:

1) always name the first argument of a method "self"
2) don't modify self in a method.

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

@mattpap

This comment has been minimized.

Copy link
Member

mattpap commented Jan 4, 2011

I disagree with 1) because in many cases it's more natural to not use self. Even in Python's standard library self is not always used as the first argument, e.g. in Fraction class from fractions module.

I have something else to add that is not connected with code quality directly. In commits 94eac6f36515ab7e5df856b509a029fcb4bdc545 and 121d6e7d1cced7249d7a1ce31efbd558d478823d in release0.7.0 branch I translated all raise statements from the form `raise Exception, (...)` to `raise Exception(...)` and added a quality test to check if raise statements of the former form won't appear again.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c35
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 4, 2011

What do you mean "don't modify self in a method"?

And by the way, the first argument of class methods should be "cls", not "self".

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

@rlamy

This comment has been minimized.

Copy link
Member

rlamy commented Jan 5, 2011

> What do you mean "don't modify self in a method"?

More precisely, it's "don't rebind the first argument of a method (usually 'self' or 'cls') inside the method".
It means that code like the following should be avoided:

class Something:
    def do_stuff(self):
        ...
        self = blah()

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 5, 2011

I see.  Yes, that is probably a bad idea, because then you lose access to self.  But does it really happen so often that we need to test for it?  I have never seen anything like that before, and I can't see why anyone would ever do it.

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

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Jan 5, 2011

grep for "self = " and you'll find some places where it happens in current sympy. 

I would do it when I intend to modify self and return a new value. A trivial example, but in the spirit of the above:

def foo(self):
  self += 1
  if self%2:
    return True
  return False

I don't see the point of creating a new variable when the input one will do...unless you are debugging and would always like to see what was input.

/c

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 5, 2011

Yeah, self is nothing special in Python; it's just a conventional name for the first argument of methods that is automatically given as object in object.method(…).  It's not like by doing "self = something" that you are rewriting the object itself.  So to me it's no different than overriding any other function argument variable.  

Or is there still a good reason not to do this that I am not seeing?

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

@rlamy

This comment has been minimized.

Copy link
Member

rlamy commented Jan 5, 2011

'self' isn't just any function argument, it has a (purely conventional) specific meaning. Rebinding it breaks the implicit promises carried by the name (e.g. the object named 'self' could then be an instance of a completely different class than the one where the method is defined) which causes an unnecessary cognitive burden and increases the risk of bad design decisions and bugs. The example with 'self += 1' is particularly bad since you have to think carefully whether it's just rebinding the name or altering the 'self' object.

In any case, this should be considered a guideline that can be broken if, but only if, there is a good reason to do so.

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

@rlamy

This comment has been minimized.

Copy link
Member

rlamy commented Jan 5, 2011

> I disagree with 1) because in many cases it's more natural to not use self. 

I really don't think there are "many cases". I agree it's debatable for some special methods like '__add__' but using something like "def __add__(a, b):" masks the syntactic dissymetry between the arguments in a case where it's particularly important to be aware of it due to the mismatch with the semantic symmetry between the arguments.

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

@vks

This comment has been minimized.

Copy link
Contributor

vks commented Jan 5, 2011

> like "def __add__(a, b):"

In this case I'd rather use 'self' and 'other', because you might have to normalize 'other', if you want to support more than just instances of your class.

Actually, right now I cannot think of an obvious case where you should not name it 'self'.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c43
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 5, 2011

> In any case, this should be considered a guideline that can be broken if, but only if, there is a good reason to do so.

OK. I agree.  So this means that it should *not* go in the quality testing (but we should be on the lookout for it when reviewing code).

As for self in the first argument, I have no opinion.  I will let you guys battle that one out :)

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 7, 2011

**Labels:** -Milestone-Release0.7.0 Milestone-Release0.7.1  

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jul 16, 2011

I'd like to do a small 0.7.1 release with IPython 0.11 support, so these will be postponed until 0.7.2.

**Labels:** Milestone-Release0.7.2  

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

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Nov 16, 2011

This is related to issue 4555 .

**Labels:** -Milestone-Release0.7.2  

Referenced issues: #4555
Original comment: http://code.google.com/p/sympy/issues/detail?id=1757#c47
Original author: https://code.google.com/u/asmeurer@gmail.com/

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016

Add regression tests & mention closed issues
    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

Add regression tests & mention closed issues
    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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment