Opened 4 years ago
Closed 4 years ago
#22160 closed enhancement (fixed)
Use centralized references in ClusterSeed
Reported by:  etn40ff  Owned by:  

Priority:  major  Milestone:  sage7.6 
Component:  documentation  Keywords:  documentation, days82 
Cc:  gmoose05, drupel, stumpc5, tscrim, egunawan  Merged in:  
Authors:  Salvatore Stella  Reviewers:  Emily Gunawan, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  b787c1c (Commits, GitHub, GitLab)  Commit:  b787c1cfa8303a5302fe3998eaa270a2126ac9fe 
Dependencies:  Stopgaps: 
Description (last modified by )
While reviewing #21254 egunawan found a double reference due to the fact that ClusterSeed
still has references defined locally.
This ticket fixes the issue.
Change History (28)
comment:1 Changed 4 years ago by
 Branch set to public/ticket/22160
 Commit set to 04f4244474d651adc9eacfa1303bba31e591900f
comment:2 Changed 4 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:3 Changed 4 years ago by
 Commit changed from 04f4244474d651adc9eacfa1303bba31e591900f to 175b53aa7054b9483f12747da3b27dc4b8ee32af
Branch pushed to git repo; I updated commit sha1. New commits:
175b53a  I can\'t even remember the lexicographic order!

comment:4 Changed 4 years ago by
 Keywords days82 added
 Status changed from needs_review to needs_info
There are several more local references in cluster_seed.py. We should move them to index.rst as well?
comment:5 Changed 4 years ago by
I think we should just move them all at once.
comment:6 Changed 4 years ago by
Agreed, I did not notice them. I'll work on it in one hour or so.
comment:7 Changed 4 years ago by
 Commit changed from 175b53aa7054b9483f12747da3b27dc4b8ee32af to 976b736920f2ddbeb473440d7e71d304e0d885fa
Branch pushed to git repo; I updated commit sha1. New commits:
976b736  Moved all references

comment:8 Changed 4 years ago by
 Status changed from needs_info to needs_review
comment:9 Changed 4 years ago by
 Commit changed from 976b736920f2ddbeb473440d7e71d304e0d885fa to 474750e709dfb6d23b5eb99d856690fa4e74c4fb
Branch pushed to git repo; I updated commit sha1. New commits:
474750e  Added missing references

comment:10 Changed 4 years ago by
There is a typo but I am recompiling on a different branch and I can't fix it right now. I should be able to mend the problem as soon as the documentation is done building.
comment:11 Changed 4 years ago by
 Commit changed from 474750e709dfb6d23b5eb99d856690fa4e74c4fb to b3ced177ba29c0b81c3bc0b6071fdc7d5e27ac9d
Branch pushed to git repo; I updated commit sha1. New commits:
b3ced17  Fix typo

comment:12 Changed 4 years ago by
Fixed, it should be good now.
comment:13 Changed 4 years ago by
 Component changed from PLEASE CHANGE to documentation
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:14 Changed 4 years ago by
 Milestone changed from sage7.5 to sage7.6
 Reviewers changed from Travis Scrimshaw to Emily Gunawan, Travis Scrimshaw
comment:15 Changed 4 years ago by
 Status changed from positive_review to needs_info
There are other references to actual websites (or arxiv numbers) in cluster_seed.py and other parts of the cluster_algebra_quiver folder. This will not cause issue for Sage, but, since this ticket was already opened, should we move those local references as well? If you don't think we should do that, you can set this back to positive_review.
comment:16 Changed 4 years ago by
 Commit changed from b3ced177ba29c0b81c3bc0b6071fdc7d5e27ac9d to 29cc90a37869d74e20f5f5c05a4c04a5812b3b2c
comment:17 Changed 4 years ago by
 Status changed from needs_info to needs_review
I now changed also all the references in every file under the folder cluster_algebra_quiver. They were in a messy variety of different styles.
comment:18 Changed 4 years ago by
 Status changed from needs_review to needs_work
We should be good and make sure those references have full bib information. For example, I suspect the Speyer reference has been published in a journal.
comment:19 Changed 4 years ago by
I already did that. I could not find better references for [Spe2013]_ nor for [Ke2008]_. I did fix many of the others.
comment:20 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:21 Changed 4 years ago by
Okay, thanks. I will let Emily set it back to a positive review of there is nothing else.
comment:22 Changed 4 years ago by
 Commit changed from 29cc90a37869d74e20f5f5c05a4c04a5812b3b2c to c71e227f6808318cc36feab49c2b0ed1ad8b009d
Branch pushed to git repo; I updated commit sha1. New commits:
c71e227  22160: fix broken reference to see sage.graphs.generic_graph.GenericGraph.canonical_label()

comment:23 Changed 4 years ago by
Salvatore, you can set this to positive_review if you're OK with the change I just did (fixing a broken reference to GenericGraph?.canonical_label).
comment:26 Changed 4 years ago by
 Commit changed from c71e227f6808318cc36feab49c2b0ed1ad8b009d to b787c1cfa8303a5302fe3998eaa270a2126ac9fe
Branch pushed to git repo; I updated commit sha1. New commits:
49dc8b8  Use centralized references

318d348  I can\'t even remember the lexicographic order!

71149f7  Moved all references

dbf7100  Added missing references

591fbc0  Fix typo

2b395ee  Changed all the remaining references in the cluster_algebra_quiver folder

4a0c541  Minor fixes

be62a45  22160: fix broken reference to see sage.graphs.generic_graph.GenericGraph.canonical_label()

b787c1c  Fixed merge conflict

comment:27 Changed 4 years ago by
 Status changed from needs_work to positive_review
I rebased the branch to the current develop. This and some other ticket introduced two new references in the same place generating the conflict. It should be good now.
comment:28 Changed 4 years ago by
 Branch changed from public/ticket/22160 to b787c1cfa8303a5302fe3998eaa270a2126ac9fe
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Use centralized references