Opened 8 months ago

Closed 8 months ago

#27072 closed enhancement (fixed)

py3: final fix for species

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.7
Component: python3 Keywords:
Cc: tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: aba9cd9 (Commits) Commit: aba9cd9754a3db1edb37bad7d41b12d046f1ada5
Dependencies: Stopgaps:

Description


Change History (14)

comment:1 Changed 8 months ago by chapoton

  • Branch set to u/chapoton/27072
  • Commit set to 0d4a59778dc4d04dcc728e219a2d60b8fe525835
  • Status changed from new to needs_review

New commits:

​0d4a597py3: final fix for species

comment:2 Changed 8 months ago by chapoton

  • Cc tscrim added

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

comment:3 Changed 8 months ago by tscrim

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
Last edited 8 months ago by tscrim (previous) (diff)

comment:4 Changed 8 months ago by git

  • Commit changed from 0d4a59778dc4d04dcc728e219a2d60b8fe525835 to b579370f31ae0849b5c84b440df7d3b11fa344c6

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

​b579370fix

comment:5 Changed 8 months ago by chapoton

done, thanks

comment:6 Changed 8 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thank you.

comment:7 Changed 8 months ago by vbraun

  • Status changed from positive_review to needs_work

Numerous doctest failures...

comment:8 Changed 8 months ago by git

  • Commit changed from b579370f31ae0849b5c84b440df7d3b11fa344c6 to acf0b7a6f205418177db45bc197b7bf47aa9afa9

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

​acf0b7apy3: final fix for species

comment:9 Changed 8 months ago by chapoton

  • Status changed from needs_work to needs_review

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

comment:10 Changed 8 months ago by tscrim

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.

comment:11 Changed 8 months ago by git

  • Commit changed from acf0b7a6f205418177db45bc197b7bf47aa9afa9 to aba9cd9754a3db1edb37bad7d41b12d046f1ada5

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

​aba9cd9py3: final fix for species

comment:12 Changed 8 months ago by chapoton

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

comment:13 Changed 8 months ago by chapoton

  • Status changed from needs_review to positive_review

ok, one green bot (the other is broken)

comment:14 Changed 8 months ago by vbraun

  • Branch changed from u/chapoton/27072 to aba9cd9754a3db1edb37bad7d41b12d046f1ada5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.