Methods concerning cores in Partitions

[anneschilling]

The patch implements some new methods regarding cores in Partitions.

---

Apply attachment: trac_11700-partitions-as.patch

CC: @zabrocki

Component: combinatorics

Keywords: partitions, cores

Author: Anne Schilling

Reviewer: Mike Zabrocki

Merged: sage-4.7.2.alpha3

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

[anneschilling]

Author: Anne Schilling

[zabrocki]

comment:2

I ran all tests and they passed. I give it a positive review except for a suggested change to is_core that I believe is shorter and faster:

def is_core( self, k ):
    return not k in self.hooks()

This changes the behavior for k=0 since the original version raises a "ZeroDivisionError: Integer modulo by zero" while this new version simply returns False.

[anneschilling]

Reviewer: Mike Zabrocki

[anneschilling]

comment:3

Hi Mike,

Thanks for your very first patch review! The suggested change is made (though for k=0 the output is True and not False).

Anne

[anneschilling]

Attachment: trac_11700-partitions-as.patch.gz

[anneschilling]

comment:7

Replying to @nthiery:

I changed the header of the patch.

[nexttime]

comment:8

Replying to @anneschilling:

Replying to @nthiery:

I changed the header of the patch.

I.e., just the commit message? (It's not clear to what you replied, Nicolas just removed the hint on the repo I previously added, for whatever reason.)

(I actually already merged the patch into a preliminary alpha3 a few hours ago, before you updated it. But shouldn't be a problem, at least if there are no "real" changes. I'll grab the latest version for an official release anyway.)

[nthiery]

comment:9

Replying to @nexttime:

Replying to @anneschilling:

I changed the header of the patch.

I.e., just the commit message?

Yup.

It's not clear to what you replied, Nicolas just removed the hint on the repo I previously added, for whatever reason.

The testbot error log was not clear, but hinted that it had not found
the patch on the ticket page. So I removed the comment after the
"Apply ..." in case this was the cause. Apparently the buildbot now
applies the patch (with test failures however). I do not know whether
this is just a coincidence or not.

Cheers,
Nicolas

[nexttime]

comment:10

Replying to @nthiery:

Replying to @nexttime:

It's not clear to what you replied, Nicolas just removed the hint on the repo I previously added, for whatever reason.

The testbot error log was not clear, but hinted that it had not found
the patch on the ticket page. So I removed the comment after the
"Apply ..." in case this was the cause.

That was my guess...

So the buildbot deletes some previous logs? The last "apply failed" is from August.

The last statement I recall regarding what the patch-/buildbot inspects was that it doesn't look at the ticket's description at all (but in contrast Jeroen's tools do; just as mine). Haven't kept track on that though.

Apparently the buildbot now
applies the patch (with test failures however).

The doctest errors are quite funny; at least one caused by the buildbot itself, most (or all?) of the others caused by "no space left on device". :D

But it IMHO doesn't make much sense to test patches still against 4.7.1 anyway.

[nexttime]

Merged: sage-4.7.2.alpha3

[anneschilling]

comment:12

Replying to @nexttime:

Sorry, I was out of e-mail contact today. Given the last message, I suppose everything was ok and merged? I only changed the very first line of the patch to

#11700: New methods concerning cores in Partitions

by an e-mail request from Nicolas.