#26263 closed defect (fixed)
py3: minor fixes for sage.rings.number_field
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-8.4 |
Component: | python3 | Keywords: | |
Cc: | chapoton | Merged in: | |
Authors: | Erik Bray | Reviewers: | Frédéric Chapoton |
Report Upstream: | N/A | Work issues: | |
Branch: | f5d4feb (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
These are the remaining fixes to this package in my python3 branch that haven't been upstreamed yet.
Apparently I already had a hash implementation for the Order
class, but this supplements the docstring with additional tests I had written.
Regarding the timestr()
function, I would normally be wary of removing any non-underscored function without a deprecation, but in this case I believe it is justified:
- It was only ever used as a utility function in a different module that was removed 8 years ago (#9359), at which point it became dead code. It should have been removed with the rest of #9359.
- Nothing else uses or has ever used it.
- It doesn't have anything to do with the rest of the surrounding code; there's no reason anyone would look to that module for such a function.
- It is not well implemented even for what it's supposed to do. In the remote chance that someone is actually using this in their code they almost certainly have bigger problems ;)
Change History (4)
comment:1 Changed 4 years ago by
- Cc chapoton added
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_review to positive_review
comment:3 Changed 4 years ago by
- Branch changed from u/embray/python3/sage-rings-number_field/misc to f5d4feb3f90d09e84f5ab4ed8fa8327460165206
- Resolution set to fixed
- Status changed from positive_review to closed
comment:4 Changed 4 years ago by
- Commit f5d4feb3f90d09e84f5ab4ed8fa8327460165206 deleted
Ack, I missed a range(...) that needed updating, but was not covered by the tests in that module. I'll just include it with my next update...
Note: See
TracTickets for help on using
tickets.
ok, let it be