add is_supergreedy() to linear extensions #34970
Conversation
|
Hello everyone, I have created this pull request in view of changes committed by me in the open issue #24700 , I had a positive review on it. Please tell if anything else needs to be done to merge this pull request. |
Sandstorm831
changed the title
|
for now, we keep the rule that only the release manager merges positively reviewed PRs. |
|
No problem, I will wait. ThankYou @dimpase for the information |
Codecov ReportBase: 88.57% // Head: 88.59% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #34970 +/- ##
===========================================
+ Coverage 88.57% 88.59% +0.01%
===========================================
Files 2140 2140
Lines 397273 397265 -8
===========================================
+ Hits 351891 351945 +54
+ Misses 45382 45320 -62
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Sir, can you tell what are the changes that I have to do. |
|
Click on "Details" for the failing Lint/Code style check. You'll see https://github.com/sagemath/sage/actions/runs/4123554420/jobs/7121712300 pointing to what you have to fix (trailing whitespace in few lines in your code here) |
…m831/sage into u/gh-Sandstorm831/24700
|
I have fixed the codestyle issues and now the test is passing, please tell if any other change needs to be done |
|
@mantepse, I have incorporated all the changes, on looking closely, your code seems easy to understand then mine, so I have updated the code. Please check and let me know if there are any more changes needs to be incorporated. |
|
One final thing: in the docstring, the definition is not very clear, in particular, the |
|
@mantepse , I have improved the definition in the docstrings. Please have a look and let me know if there any more changes needs to be done. |
This test failed. |
Sandstorm831
requested review from
mantepse
and removed request for
dimpase,
tobiasdiez and
mantepse
|
@mantepse, done the corrections |
|
Dear @Sandstorm831, please excuse my being pedantic, I noticed another slight simplification of the code. It is nothing spectacular, but I think it makes the intent clearer. Warning: we now need to produce if not self:
return True
H = self.poset().hasse_diagram()
L = sources = H.sources()
linext = []
for e in self:
if e not in L:
return False
linext.append(e)
for y in reversed(linext):
L = [x for x in H.neighbor_out_iterator(y)
if x not in linext
and all(low in linext for low in H.neighbor_in_iterator(x))]
if L:
break
else:
L = sources = [x for x in sources if x not in linext]
return True |
|
In principle we could remove the first two lines, checking whether |
|
@dimpase @mantepse sorry if I am missing something, but I want to ask how we avoided computing the poset, hasse diagram and sources. I can see that we have to calculate all poset, it's hasse diagram and all sources, on the contrary I can't see how this is optimization over the previous code, as I can notice that the check which was previously present at the bottom, shifted above and L had been initialised earlier. |
|
I wasn't saying that is an optimization, except that the code is now shorter. Instead of checking |
|
Ohk, sorry @mantepse I misunderstood your point, I will change the code asap. |
|
Documentation preview for this PR is ready! 🎉 |
This is my pull request originally created on trac, which I thought pushing on github.
Fixes #24700