Opened 3 years ago
Closed 3 years ago
#23955 closed defect (fixed)
trains() forgets the trains of length one.
| Reported by: | sbrandhorst | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | sage-8.1 |
| Component: | quadratic forms | Keywords: | sd91 |
| Cc: | Merged in: | ||
| Authors: | Simon Brandhorst | Reviewers: | David Roe |
| Report Upstream: | N/A | Work issues: | |
| Branch: | d7fa2c4 (Commits) | Commit: | d7fa2c47e7e39210e59944c71e10f949bc329999 |
| Dependencies: | Stopgaps: |
Description
In sage.modules.genera.genus.canonical_2_adic_trains forgets the trains of length one. Maybe this causes further bugs.
Change History (9)
comment:1 Changed 3 years ago by
- Component changed from PLEASE CHANGE to quadratic forms
- Keywords sd91 added
comment:2 Changed 3 years ago by
- Branch set to u/sbrandhorst/trains___forgets_the_trains_of_length_one_
comment:3 Changed 3 years ago by
- Commit set to 711bbf05fd5d2d805d609b44f3b1a559f1e0c946
- Status changed from new to needs_review
comment:4 Changed 3 years ago by
- Branch changed from u/sbrandhorst/trains___forgets_the_trains_of_length_one_ to u/roed/trains___forgets_the_trains_of_length_one_
comment:5 Changed 3 years ago by
- Commit changed from 711bbf05fd5d2d805d609b44f3b1a559f1e0c946 to 8e8c9cca6f92fd5fcc268c3d9b456b4039c6a1fa
- Reviewers set to David Roe
Most of my changes were superficial, but I did find what I thought was a typo. You used to have
if left_neighbor == 1 or symbol[i] == 1 or right_neighbor==1:
which I changed to
if left_neighbor == 1 or symbol[i][3] == 1 or right_neighbor==1:
based on context. If this is correct, you're happy with the rest of my changes, and tests pass, positive review.
New commits:
| 43cfcc1 | Merge branch 'u/sbrandhorst/trains___forgets_the_trains_of_length_one_' of git://trac.sagemath.org/sage into t/23955/trains
|
| 8e8c9cc | Small fixes to new trains code
|
comment:6 Changed 3 years ago by
- Status changed from needs_review to positive_review
Thank you for the changes. It is a lot cleaner now. Tests pass on my machine and the patchbot only complains about long tests in other unrelated components. Seems good.
comment:7 Changed 3 years ago by
- Commit changed from 8e8c9cca6f92fd5fcc268c3d9b456b4039c6a1fa to d7fa2c47e7e39210e59944c71e10f949bc329999
- Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
| d7fa2c4 | Fix tab character in index.rst
|
comment:8 Changed 3 years ago by
- Status changed from needs_review to positive_review
No, that was a real problem in the tests with a TAB character. I've fixed it now.
comment:9 Changed 3 years ago by
- Branch changed from u/roed/trains___forgets_the_trains_of_length_one_ to d7fa2c47e7e39210e59944c71e10f949bc329999
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Rewrote canonical_2_adic_trains()Removed unnecessary computations of compartments for the train function.