Closed nthiery closed 9 years ago
Patch under development on the Sage-Combinat patch server.
Description changed:
---
+++
@@ -1 +1,3 @@
+At Sage days 30, a long brainstorm seems to have finalized the design. I'm writing a patch on sage combinat queue.
+
Changed keywords from none to factories, days30
Changed keywords from factories, days30 to factories, days30, Cernay2012
Description changed:
---
+++
@@ -1,3 +1,29 @@
-At Sage days 30, a long brainstorm seems to have finalized the design. I'm writing a patch on sage combinat queue.
+At Sage days 30, a long brainstorm seems to have finalized the design. Here is a exerpt from the documentation:
-
+A *set factory* `F` is device, whose goal is to construct parent `P`
+which models subsets of a big set `S`. Typically, the `P` s are constructed
+within families obtained by putting a bunch of constraints `cons` on the
+elements of the set `S`. In such a hierarchy of subsets, one needs to have a
+fine and easy control on the elements construction. That is, one often needs
+that `P` constructs elements in a subclass of its usual class for element. On
+the contrary, one also often needs `P` to be a facade parent, meaning that `P`
+construct element whose actual parent is not `P` itself.
+
+The role of a set factory is twofold:
+
+- *manage a database* of constructors for the different parents `P = F(cons)`
+ depending on the various kinds of constraints `cons`. Note: currently there
+ is no real support for that. We are gathering use case before fixing the
+ interface.
+
+- ensure that the elements `e = P(...)` created by the different parents
+ follows a consistent policy concerning their *class and parent*.
+
+The patch implement this idea while trying to leave as much as possible space
+for further improvement. In particular, I tried to specify the few possible
+things about constraints. I'm even not completely sure about `add_constraints`.
+Please comment and review.
+
+Florent
+
+**Apply :** [trac_10194-factories_policy-fh.patch](https://github.com/sagemath/sage-prod/files/10651310/trac_10194-factories_policy-fh.patch.gz)
Work Issues: doctest failures
Attachment: trac_10194-factories_policy-fh.patch.gz
This fails doctests on every version tested -- see patchbot logs
Replying to @loefflerd:
This fails doctests on every version tested -- see patchbot logs
I forgot a dependency to #10963. Thanks for pointing this out.
Dependencies: #10963
There is no patch at #10963. I don't think this can be meaningfully tagged as "needs review" when one of its dependencies doesn't exist yet.
Replying to @loefflerd:
There is no patch at #10963. I don't think this can be meaningfully tagged as "needs review" when one of its dependencies doesn't exist yet.
As you can see on #10963, the patch is far for being inexistent as Simon and Christian started to review it. Of course it is on the Sage-Combinat queue. I need to ping Nicolas to have him finish it. Now, I don't see why having Nicolas's patch unfinished forbid me to have some feedback on this one. And asking for such a feedback seems to be a perfectly meaningful reason for putting "needs review"... The only reason I can see for not doing it is that it can trouble the patchbot. Do you have another one ?
Salut Florent!
Why is there a dependency on 10963? In the queue, this patch is much lower than 10963.
Ah, I see, it's about the glitch Sets().Facade() <-> Sets().Facades().
Since anyway the patch is up the queue, I suggest we finish commuting the two patches. Just use Sets().Facades() (we will need to keep the backward compatibility anyway).
With that, the tests pass up to to trivially failing doctests:
File "/opt/sage-5.0.beta11/devel/sage-combinat/sage/structureset_factories.py", line 644:
sage: P2.category()
Expected:
Join of Category of facade finite enumerated sets and Policy category with facade parent AllPairs
Got:
Join of Category of finite enumerated sets and Policy category with facade parent AllPairs
**********************************************************************
File "/opt/sage-5.0.beta11/devel/sage-combinat/sage/structure/set_factories.py", line 972:
sage: P.category()
Expected:
Join of Category of facade finite enumerated sets and Policy category with facade parent AllPairs
Got:
Join of Category of finite enumerated sets and Policy category with facade parent AllPairs
Cheers,
Attachment: trac_10194-factories_policy-fh-v2.patch.gz
new version, rebased on 5.12.beta0, with cleanup
I have made a git branch, but it seems to be massively broken.
Some problems come from Sets().Facade() versus Sets().Facades() as explained before
Some other problems come from #14519
And some other problems I do not understand..
New commits:
5dbd72b | trac_10194-factories_policy-fh.patch |
Branch: u/chapoton/10194
Branch pushed to git repo; I updated commit sha1. New commits:
57f3ed3 | trac #10194 minor changes |
Branch pushed to git repo; I updated commit sha1. New commits:
28c42cb | trac #10194 typo corrections |
Temporary : emptied dependency field (was #10963)
Changed dependencies from #10963 to none
Branch pushed to git repo; I updated commit sha1. New commits:
6fa45d8 | trac #10194 other details, still not working :( |
This needs to be worked upon, as it is not working at all.
Maybe some of the problems come from #13801 ?
Changed branch from u/chapoton/10194 to u/hivert/10194
Branch pushed to git repo; I updated commit sha1. New commits:
06beedc | Fixed passing parent to ElementWrapper |
Changed keywords from factories, days30, Cernay2012 to factories, days30, Cernay2012, days57
Branch pushed to git repo; I updated commit sha1. New commits:
d7eb4c1 | Cleanup in constraints managment. |
Branch pushed to git repo; I updated commit sha1. New commits:
b615150 | Fixed printing of `_repr_` for TopMostParentPolicy |
Maybe you should add #13580 as a dependency ?
Replying to @fchapoton:
Maybe you should add #13580 as a dependency ?
Why ? Those are completely different things.
Then why do you have that in the commits of this ticket ?
Replying to @fchapoton:
Then why do you have that in the commits of this ticket ?
I probably screwed things up with git ! I'm trying to fix the problem.
For information, this features is largely tested in #16094
Changed branch from u/hivert/10194 to u/hivert/10194-rebased
Should be fixed now ! But I had to rebase the branch. So Nicolas, you will have to rebase yours at some point. Sorry !
New commits:
dc5dfc8 | # Mon Apr 07 00:47:29 2014 +0200 |
e595280 | Fixed passing parent to ElementWrapper |
80b08c3 | Cleanup in constraints managment. |
fc6981b | Fixed printing of `_repr_` for TopMostParentPolicy |
Changed branch from u/hivert/10194-rebased to u/hivert/10194-rebased-beta7
Changed branch from u/hivert/10194-rebased-beta7 to u/nborie/10194-rebased-beta7
I have made one more commit. Now all tests pass, and code is in the pep8 standard.
New commits:
77279f9 | # Mon Apr 07 00:47:29 2014 +0200 |
272d17f | Fixed passing parent to ElementWrapper |
1491bff | Cleanup in constraints managment. |
41c137e | Fixed printing of `_repr_` for TopMostParentPolicy |
f76e0e6 | Review : fonctionality, documentations, tests (very small things remains...) |
1f3e02b | Merge branch 'u/nborie/10194-rebased-beta7' of trac.sagemath.org:sage into 10194 |
0eeecd1 | trac #10194 make sure that all test pass (and pyflakes and pep8 compliant) |
Changed branch from u/nborie/10194-rebased-beta7 to public/ticket/10194
There remains a pyflakes warning thta needs to be taken care of :
src/sage/structure/set_factories.py:722: undefined name 'FacadeParentPolicyCategory'
Changed work issues from doctest failures to pyflakes warning
At Sage days 30, a long brainstorm seems to have finalized the design. Here is a excerpt from the documentation:
A set factory
F
is device, whose goal is to construct parentP
which models subsets of a big setS
. Typically, theP
s are constructed within families obtained by putting a bunch of constraintscons
on the elements of the setS
. In such a hierarchy of subsets, one needs to have a fine and easy control on the elements construction. That is, one often needs thatP
constructs elements in a subclass of its usual class for element. On the contrary, one also often needsP
to be a facade parent, meaning thatP
construct element whose actual parent is notP
itself.The role of a set factory is twofold:
manage a database of constructors for the different parents
P = F(cons)
depending on the various kinds of constraintscons
. Note: currently there is no real support for that. We are gathering use case before fixing the interface.ensure that the elements
e = P(...)
created by the different parents follows a consistent policy concerning their class and parent.The patch implement this idea while trying to leave as much as possible space for further improvement. In particular, I tried to specify the few possible things about constraints. I'm even not completely sure about
add_constraints
. Please comment and review.Florent
CC: @sagetrac-sage-combinat @hivert
Component: combinatorics
Keywords: factories, days30, Cernay2012, days57
Author: Florent Hivert
Branch/Commit:
763f93c
Reviewer: Frédéric Chapoton
Issue created by migration from https://trac.sagemath.org/ticket/10194