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

py3: final fix for species #27072

Closed
fchapoton opened this issue Jan 17, 2019 · 20 comments
Closed

py3: final fix for species #27072

fchapoton opened this issue Jan 17, 2019 · 20 comments

Comments

@fchapoton
Copy link
Contributor

CC: @tscrim

Component: python3

Author: Frédéric Chapoton

Branch/Commit: aba9cd9

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/27072

@fchapoton fchapoton added this to the sage-8.7 milestone Jan 17, 2019
@fchapoton
Copy link
Contributor Author

Commit: 0d4a597

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/27072

@fchapoton
Copy link
Contributor Author

New commits:

0d4a597py3: final fix for species

@fchapoton
Copy link
Contributor Author

comment:2

green bot (damn bot with no pyflakes..), please review

@tscrim
Copy link
Collaborator

tscrim commented Jan 17, 2019

comment:3

Perhaps it would be better to have min=0 be the default, and so we can change this test:

-        if ((min is None and max is None) or
-            (max is None and self.get_aorder() >= min)):
+        if (max is None and (not min or self.get_aorder() >= min)):
             return self

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 17, 2019

Changed commit from 0d4a597 to b579370

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 17, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

b579370fix

@fchapoton
Copy link
Contributor Author

comment:5

done, thanks

@tscrim
Copy link
Collaborator

tscrim commented Jan 17, 2019

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jan 17, 2019

comment:6

Thank you.

@vbraun
Copy link
Member

vbraun commented Jan 25, 2019

comment:7

Numerous doctest failures...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2019

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

acf0b7apy3: final fix for species

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2019

Changed commit from b579370 to acf0b7a

@fchapoton
Copy link
Contributor Author

comment:9

indeed. Let us step back to my basic but working fix.

@tscrim
Copy link
Collaborator

tscrim commented Jan 29, 2019

comment:10

Dumb thing, but this change makes it overindented:

-            (max is None and self.get_aorder() >= min)):
-            return self
+             (max is None and self.get_aorder() >= min)):
+             return self

Once fixed and green bot (either before or after), you can set back to a positive review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2019

Changed commit from acf0b7a to aba9cd9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 29, 2019

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

aba9cd9py3: final fix for species

@fchapoton
Copy link
Contributor Author

comment:12

right. Done. I will re-launch my bot on it.

@fchapoton
Copy link
Contributor Author

comment:13

ok, one green bot (the other is broken)

@vbraun
Copy link
Member

vbraun commented Feb 4, 2019

Changed branch from u/chapoton/27072 to aba9cd9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants