IncidenceStructure.__contains__

[nathanncohen]

Right now it is obtained through .__iter__, and it does not do the job

sage: [1,2,3,4] in IncidenceStructure([[1,2,3,4]])
True
sage: [4,3,2,1] in IncidenceStructure([[1,2,3,4]])
False

CC: @videlec @KPanComputes @dimpase

Component: combinatorial designs

Author: Nathann Cohen

Branch/Commit: c69307c

Reviewer: Dima Pasechnik

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

[nathanncohen]

Branch: u/ncohen/16727

[sagetrac-git]

Commit: 1dd244a

[sagetrac-git]

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

1dd244a

trac #16727: IncidenceStructure.__contains__

[dimpase]

comment:3

at least some tests should use some "real" incidence structures, not just 1-element examples...

[sagetrac-git]

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

e7ef8c0

trac #16727: Moloch! Time Eater!

[sagetrac-git]

Changed commit from 1dd244a to e7ef8c0

[dimpase]

comment:5

Replying to @sagetrac-git:

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

e7ef8c0

trac #16727: Moloch! Time Eater!

Well, test on non-degenerate examples, please! Your favourite STS on 15 points will do for sure...

[nathanncohen]

comment:6

Well, test on non-degenerate examples, please! Your favourite STS on 15 points will do for sure...

Hey Dima I have no idea what doctest you want me to write but I am not going to lose 10 minutes per try until I find out. Add a commit to the branch if you don't like my doctests.

Nathann

[nathanncohen]

Changed branch from u/ncohen/16727 to public/16727

[dimpase]

comment:7

Replying to @nathanncohen:

Well, test on non-degenerate examples, please! Your favourite STS on 15 points will do for sure...

Hey Dima I have no idea what doctest you want me to write but I am not going to lose 10 minutes per try until I find out. Add a commit to the branch if you don't like my doctests.

A degenerate input is a corner case: one-element hypergraphs and complete uniform hypergraphs are surely degenerate inputs. Such tests don't stress much of the implementation, as you can imagine.
Test on, say, ProjectiveGeometryDesign(3,1,GF(2)) as well.

[nathanncohen]

comment:8

A degenerate input is a corner case: one-element hypergraphs and complete uniform hypergraphs are surely degenerate inputs. Such tests don't stress much of the implementation, as you can imagine.

Dima, are you aware that the only thing we are testing here is whether an element belongs to a LIST ? Python does not care at all what the elements are, it is all pointers as far as it is concerned.

Test on, say, ProjectiveGeometryDesign(3,1,GF(2)) as well.

Add your commit.

Nathann

[dimpase]

comment:9

Replying to @nathanncohen:

A degenerate input is a corner case: one-element hypergraphs and complete uniform hypergraphs are surely degenerate inputs. Such tests don't stress much of the implementation, as you can imagine.

Dima, are you aware that the only thing we are testing here is whether an element belongs to a LIST ?

Good doctests should test the function as if it were a blackbox.
The underlying implementation might change, might become more sophisticated, and the doctests should catch bugs in such future changes. Unless you prefer bug-driven development model to the test-driven one ;-)

[nathanncohen]

comment:10

Good doctests should test the function as if it were a blackbox.

Religion does not work on me.

Unless you prefer bug-driven development model to the test-driven one ;-)

I am not against doctests, I am against your spending my time on this. I believe that this function is sufficiently doctested, and I already added tests because you asked. If they don't satisfy you for some reason, I do not mind it but you will have to write your own and add it to the branch.

Nathann

[dimpase]

comment:11

Replying to @nathanncohen:

Good doctests should test the function as if it were a blackbox.

Religion does not work on me.

It has nothing to do with religion, this is just common sense, sorry.

Unless you prefer bug-driven development model to the test-driven one ;-)

I am not against doctests, I am against your spending my time on this. I believe that this function is sufficiently doctested, and I already added tests because you asked. If they don't satisfy you for some reason, I do not mind it but you will have to write your own and add it to the branch.

I believe that I explained sufficiently clear that testing only corner cases does not work, yet you added another corner case test, for a complete uniform hypergraph is a corner case, AFAIK.

And I have elected to be a reviewer, sorry. It is my job to decide what a sufficient doctest is, not yours.
Well, if you cannot add the doctests I ask for, I can surely do this too, only then you will be spending my time on this, not the other way around.

[nathanncohen]

comment:12

Yo !

I believe that I explained sufficiently clear that testing only corner cases does not work, yet you added another corner case test, for a complete uniform hypergraph is a corner case, AFAIK.

A uniform hypergraph is a "corner case" in hypergraph theory. Not as an example of Python list. I mixed integers and strings, and order. That's how I tested non-corner cases that made sense from the point of view of what this function does. It is also a doctest of the bug I reported in this ticket's description.

And I have elected to be a reviewer, sorry. It is my job to decide what a sufficient doctest is, not yours.

Indeed, but when you see that you cannot explain what you want without making lose the other guy's time because he apparently does not get it, you can save everybody by writing the 4 lines yourself.

Nathann

[dimpase]

comment:13

Replying to @nathanncohen:

Yo !

I believe that I explained sufficiently clear that testing only corner cases does not work, yet you added another corner case test, for a complete uniform hypergraph is a corner case, AFAIK.

A uniform hypergraph is a "corner case" in hypergraph theory. Not as an example of Python list. I mixed integers and strings, and order. That's how I tested non-corner cases that made sense from the point of view of what this function does. It is also a doctest of the bug I reported in this ticket's description.

And I have elected to be a reviewer, sorry. It is my job to decide what a sufficient doctest is, not yours.

Indeed, but when you see that you cannot explain what you want without making lose the other guy's time because he apparently does not get it, you can save everybody by writing the 4 lines yourself.

OK, will do...

Nathann

[dimpase]

comment:14

Replying to @dimpase:

OK, will do...

after I'm done building 6.3.beta7, that is...

[sagetrac-git]

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

c69307c

added a non-generic examples, and some minor changes

[sagetrac-git]

Changed commit from e7ef8c0 to c69307c

[nathanncohen]

comment:16

Yoooooooooooooooo !

The tests pass, so if that's okay for you... :-)

Nathann

[dimpase]

Reviewer: Dima Pasechnik

[vbraun]

Changed branch from public/16727 to c69307c