Opened 11 years ago
Closed 10 years ago
#7978 closed enhancement (fixed)
Cleanup of crystal code
Reported by: | jbandlow | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3.3 |
Component: | combinatorics | Keywords: | crystals |
Cc: | sage-combinat | Merged in: | sage-4.3.3.alpha0 |
Authors: | Nicolas M. Thiéry, Anne Schilling | Reviewers: | Dan Bump |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
Fixed some issues in crystals, such as
- Comparison of elements
- Latex output
- Fixes whitespaces in /combinat/crystals/affine.py (as reported by Jason Bandlow)
- All crystals have unique representation
- Preparation of categorification of crystals:
- C.element_class -> C.Element
- All crystals are at least in the EnumeratedSets? category
- Use rename, or define _repr_, instead of setting _name Eventually, crystals should only use _repr_ => Removed dependency upon deprecated CombinatorialClass?
- Systematic use of TestSuite? instead of loads/dumps test
- Fixed bug in fast_crystal: delpat is not immutable (which should be eventually be fixed), and was accidentally changed by the weight method). This was caught by turning on unique representation which made the crystals to be reused longer in the tests.
Depends on #8028 (element wrapper improvement)
Attachments (3)
Change History (11)
Changed 11 years ago by
comment:1 Changed 11 years ago by
- Status changed from new to needs_work
Changed 11 years ago by
comment:2 Changed 11 years ago by
- Description modified (diff)
- Keywords crystals added
- Reviewers set to Dan Bump
- Status changed from needs_work to needs_review
- Summary changed from The file sage/combinat/affine.py contains lots of 'tab' characters to Cleanup of crystal code
The patch trac_7978_crystal_cleanup-as.patch supersedes Jason's patch. It fixes the whitespace issues in combinat/crystals/affine.py and does a lot more improvements in crystals (see description).
comment:3 Changed 11 years ago by
- Description modified (diff)
- Type changed from defect to enhancement
comment:4 Changed 11 years ago by
- Status changed from needs_review to positive_review
I checked that the patch passes sage --testall, either with or without #8028.
I checked that it does not break latex method for classical crystals or metapost method for fastcrystals. I think the latex changes are specific to affine crystals.
The patch does not seem to break anything, and fixes some problems. I recommend merging it.
comment:5 Changed 10 years ago by
I've added the ticket number in the refreshed patch I'm applying to 4.3.3.alpha0.
comment:6 Changed 10 years ago by
I'm having trouble parsing mpatel's message. Does this mean the patch is to be merged in 4.3.3.alpha0?
What "refreshed patch" does this message refer to? The patch trac_7978_crystal_cleanup-as.2.patch applies cleanly to sage-4.3.2 and does not break anything in sage/combinat/crystals.
comment:7 Changed 10 years ago by
I'll merge this ticket into 4.3.3.alpha0. I mean only that I've prepended the ticket number to the first line of the commit string of the patch I've added to my working tree for 4.3.3.alpha0. I apologize for being so cryptic.
comment:8 Changed 10 years ago by
- Merged in set to sage-4.3.3.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Oops. This patch depends on some patches in the combinat stack. (I suspect the 'crystal-cleanup' patches.) The change is trivial, but I'll coordinate with the sage-combinat list before marking this ready for review.