-
-
Notifications
You must be signed in to change notification settings - Fork 553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the category of SemiRings with an example : NonNegativeIntegers() #9056
Comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Attachment: trac_9056_semirings_category-nb.2.patch.gz |
Author: Nicolas Borie |
Reviewer: Nicolas M. Thiéry |
comment:4
I reviewed this patch while Nicolas was writting it. I'll run the tests shortly, and report. Florent: please have a look to see if you agree with the concept, and if yes set a positive review. |
comment:5
Hi Nicolas B., There are a couple failures in the category tests (due to the new test and new category); please fix them. Note that I made a small change to your patch, so you should make sure to first pull. You might get a conflict with the primer improvement; in that case, move your patch just after it. |
comment:6
Replying to @nthiery:
A couple failures in combinat/sf as well due to the new _test_distributivity. |
comment:7
As I did manage to run ALL TESTS on sagemath, I found files this patch affect... I didn't touch the primer (modified on combinat queue). This patch will have yo be updated if your primer improvements go in sage earlier. I update the patch (without .2.patch, forget this one.) Thank for your help for massive tests. |
comment:8
Sorry and please wait, this local patch will break your queue Nicolas. |
comment:9
This last update should be fine... |
comment:10
One more update : After advises form English speaker people, I change the name from NonNegativeIntegersSemiring to NonNegativeIntegerSemiring. See http://groups.google.com/group/sage-devel/browse_thread/thread/ffaf01ffb941078 I linked my module to the reference manual and fix a :class: ref in the doc thanks to Florent. I am still ready for deeper review. |
This comment has been minimized.
This comment has been minimized.
comment:12
Hi Nicolas, Thanks for finalizing this! I just pushed a small reviewer's patch on sage-combinat. Please review, and if ok fold and reupload here. |
comment:13
Attachment: trac_9056_semirings_category-nb.patch.gz I am ok with your reviewer patch. I will try to delete ending blanks on my own since you made work well coloring with my hg qdiff. I qfolded your patch in mine and uploaded it... Thanks for the review. For the release manager ,apply only |
comment:14
I did not yet say to set a positive review on my behalf :-) |
comment:15
With my reviewer patch all test pass on Sage-4.4.3, with the following patches applied: trac_8704-integer_range_print-fh.patch (note: actually interfaces/expect and interfaces/ecm failed, but those seem to be usual random failures on massena which would need to be investigated at some point). Nicolas: please fold, and reupload, and set positive review on my behalf! |
comment:16
Arr, here is the list of patches in a more readable format:
|
Attachment: trac_9056-disable-llt-test.patch.gz |
comment:17
Things look good to me. Could I get a quick review of the patch I just posted. Without this, llt.py times out. |
Merged: sage-4.4.4.alpha1 |
comment:19
I've folded together the two #9056 patches on the combinat server together and merged the llt patch here. |
comment:21
Thanks Mike for the fix! For now, I still don't really manage to integrate completely such patch which touch so many things in Sage. Dependencies are not trivial when you begin to modify categories. For Nicolas Thiéry : This patch go in Sage before I fold your second reviewer patch :
I don't no the status about your improvements of the category primer but be aware about this fact. As I don't want to produce some chaos in the queue, I didn't touch your reviewer patch "trac_9056_semirings_category-review-nt.patch". Sorry to being late to fold it. Nicolas (the little). |
comment:22
Replying to @sagetrac-nborie:
Don't worry, you are doing a great job.
Mike said above that he took the two patches right away from the Sage-Combinat queue and folded them together. So all is fine. |
All is in the title, we want :
CC: @sagetrac-sage-combinat @nthiery
Component: categories
Keywords: semiring
Author: Nicolas Borie
Reviewer: Nicolas M. Thiéry
Merged: sage-4.4.4.alpha1
Issue created by migration from https://trac.sagemath.org/ticket/9056
The text was updated successfully, but these errors were encountered: