Opened 11 years ago
Closed 8 years ago
#11840 closed defect (fixed)
sage.symbolic.expression.Expression.collect_common_factors has no documentation
Reported by: | mjo | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-6.2 |
Component: | documentation | Keywords: | symbolic, beginner |
Cc: | Merged in: | ||
Authors: | Frédéric Chapoton | Reviewers: | Marc Mezzarobba, Ralf Stephan |
Report Upstream: | N/A | Work issues: | |
Branch: | fb4ca11 (Commits, GitHub, GitLab) | Commit: | fb4ca118515da0514bdaca99807dfa2876feab26 |
Dependencies: | Stopgaps: |
Description
The collect_common_factors method has an example, but no other documentation. It is not obvious from the documentation what the method is supposed to do.
Change History (20)
comment:1 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:2 Changed 9 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:3 Changed 8 years ago by
- Keywords symbolic beginner added
comment:4 Changed 8 years ago by
- Branch set to u/chapoton/11840
- Commit set to 06d4fe0e2b1c40cbebd4f72ffd171ba9b2389db6
- Status changed from new to needs_review
comment:5 Changed 8 years ago by
- Reviewers set to Marc Mezzarobba
I'd put the comment about LOG_TEN_TWO_PLUS_EPSILON
before its definition rather than below. The rest looks fine to me.
comment:6 Changed 8 years ago by
- Commit changed from 06d4fe0e2b1c40cbebd4f72ffd171ba9b2389db6 to 2b111097f96020a082f669ef426e999bd68597e7
Branch pushed to git repo; I updated commit sha1. New commits:
2b11109 | trac #11840 swapped two lines
|
comment:7 Changed 8 years ago by
Done. Many thanks for your reviews, here and in other tickets !
comment:8 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:9 Changed 8 years ago by
Conflict, please merge in latest beta
comment:10 Changed 8 years ago by
Sorry, but I do not see any conflit with 6.2.beta3. Is there any 6.2.beta4 ?
comment:11 Changed 8 years ago by
No, not yet. Conflict is then probably due to the pynac update #15198. The easiest solution will be to try again when beta4 is out.
comment:12 Changed 8 years ago by
In case it may be of use, I rebased Frédéric's branch on top of https://github.com/vbraun/sage.git/develop and uploaded the result under u/mmezzarobba/11840-collect_common_factors
.
comment:13 Changed 8 years ago by
Wait, this change will also conflict with #15892, which probably should go in first.
comment:14 Changed 8 years ago by
Please merge in 6.2.beta4
comment:15 Changed 8 years ago by
- Commit changed from 2b111097f96020a082f669ef426e999bd68597e7 to f7c9326ea9da5c30b49cb7eceae2c9dd64fc9119
- 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:
f7c9326 | Merge branch 'u/chapoton/11840' of trac.sagemath.org:sage into 11840
|
comment:16 Changed 8 years ago by
merging done
comment:17 Changed 8 years ago by
- Branch changed from u/chapoton/11840 to public/11840
- Commit changed from f7c9326ea9da5c30b49cb7eceae2c9dd64fc9119 to fb4ca118515da0514bdaca99807dfa2876feab26
Merges fine with 6.2beta5. Docs compile and look good. I have uploaded a reviewer's patch at public/11840
. If you like it, set positive. If not, set Branch:
to your branch again and set positive.
New commits:
cedda60 | Merge branch 'u/chapoton/11840' of git://trac.sagemath.org/sage into tmp
|
fb4ca11 | trac #11840: reviewer's patch: consistent use of imperative definition
|
comment:18 Changed 8 years ago by
- Reviewers changed from Marc Mezzarobba to Marc Mezzarobba, Ralf Stephan
comment:19 Changed 8 years ago by
- Status changed from needs_review to positive_review
ok, then good to me too. Setting to positive review. Thanks for the review.
comment:20 Changed 8 years ago by
- Branch changed from public/11840 to fb4ca118515da0514bdaca99807dfa2876feab26
- Resolution set to fixed
- Status changed from positive_review to closed
Here is a git branch with a little bit more documentation for this method.
I have also taken the opportunity to put raise statement into python3 format, and to use the trac role to add links to the tickets.
New commits:
trac #11840 first step, plus doc python 3 and trac role cleanup
trac #11840 details, making sure that tests pass