#19979 closed enhancement (fixed)
Improving Coding Style and Documentation in projective product schemes
Reported by: | rlmiller | Owned by: | rlmiller |
---|---|---|---|
Priority: | trivial | Milestone: | sage-7.1 |
Component: | algebraic geometry | Keywords: | |
Cc: | Merged in: | ||
Authors: | Rebecca Lauren Miller | Reviewers: | Ben Hutz |
Report Upstream: | N/A | Work issues: | |
Branch: | 7b099f7 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description
There are number of coding style issues and documentation issues in the affine scheme functionality such as
- == False or == True
- references to self in docs
- no single line description
- formatting of errors
Change History (21)
comment:1 Changed 6 years ago by
- Branch set to u/rlmiller/test
comment:2 Changed 6 years ago by
- Commit set to 8264cabe9eb30680f80026c10223e56108e4bc99
- Summary changed from Improving Codine Style and Documentation in projective schemes to Improving Coding Style and Documentation in projective product schemes
comment:3 Changed 6 years ago by
- Branch changed from u/rlmiller/test to u/rlmiller/t/19979
- Commit changed from 8264cabe9eb30680f80026c10223e56108e4bc99 to 10f66b453e67b3367aff3a9537e84c4f3e5f37bc
Something went wrong with the rebase. Here is a corrected branch.
New commits:
10f66b4 | 19979 Improve coding for product projectives
|
comment:4 Changed 6 years ago by
- Commit changed from 10f66b453e67b3367aff3a9537e84c4f3e5f37bc to a5b09747394eb66468d13eb788d32805b3da122f
Branch pushed to git repo; I updated commit sha1. New commits:
a5b0974 | Merge branch 'master' into t/19979/t/19979
|
comment:5 Changed 6 years ago by
- Status changed from new to needs_review
comment:6 Changed 6 years ago by
- Cc bhutz added
comment:7 Changed 6 years ago by
- Status changed from needs_review to needs_work
merge conflict with the latest beta.
comment:8 Changed 6 years ago by
- Commit changed from a5b09747394eb66468d13eb788d32805b3da122f to 7ddbc55b5e88bedb515f9b3db60752ad61e2e099
comment:9 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:10 Changed 6 years ago by
- Cc bhutz removed
- Reviewers set to Ben Hutz
- Status changed from needs_review to needs_work
The docs don't build:
[schemes ] /home/ben/sage/sage-test/local/lib/python2.7/site- packages/sage/schemes/product_projective/space.py:docstring of sage.schemes.product_projective.space.is_ProductProjectiveSpaces:1: WARNING: Inline interpreted text or phrase reference start-string without end-string.
wehlerK3: I started looking at this line by line, but there are many repeating issues so I'll look at this file line-by-line after you make another pass.
- no spaces after ( or before )
- bilinear is spelled wrong many times
here are the first few instances of these:
- import spacing
- line 72 bilinear misspelled
- line 75: OUTPUT: :class:
WehlerK3Surface_ring
- line 102: no space for (space
- 106: bilinear
- 110 OUTPUT: :class:
WehlerK3Surface_ring
- 118 wrap line
- 129: no spaces before/after (, )
- 135: bilinear
- 150, 159, 161, 163, 166, 168, 170, 173: paren space
- 182 output
- 199 paren space, no need for () around return
- 207
AttributeError?
, True if point is on surface
- 243 paren space
- 245 need better one line description
- 249, 251, 261, 285, 287 paren space
- 291 better one line description
Here is what I saw in the other files:
space.py:
- is_ProduceProjectiveSpaces: shorter one line description
- 137: spaces
- 428 one line
- 443 ending punc
- 447 .
- 549 onthis
- 733
R
point.py
- 82: spaces
- 99,103,107
i
- 237 thid
- 294
t
- 333: self
- 351: parameter spaces
- 373: , QQ
- 391 spaces
- 415
n
->N
, P is self, so this doc string needs to be reworded
- 417 n -> N, self/P are not right
- 424 self
- 457 spaces
- 460 codomain
- 461 N, space (
morphism.py
- 31 blank line before
- 78, 79, 90, 91: , QQ)
- 124,128,132
i
- 188 , [1,1,1]
- 226 space
- 240 iff -> if and only if
- 323 thus -> this, n -> N
- 359: just start this with "This function..."
- 364 reducing -> reduce
- 389 codomain
comment:11 Changed 6 years ago by
- Commit changed from 7ddbc55b5e88bedb515f9b3db60752ad61e2e099 to 5fe2f41978ac77f4cd45f9494b148f2b54daa76f
Branch pushed to git repo; I updated commit sha1. New commits:
5fe2f41 | 19979 Fixing style of product projective folder
|
comment:12 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:13 Changed 6 years ago by
- Branch changed from u/rlmiller/t/19979 to u/bhutz/t/19979
- Commit changed from 5fe2f41978ac77f4cd45f9494b148f2b54daa76f to 7b099f76e9e6217c03320d0c9d85f8011a1b2202
I made some small additional corrections. You should double check those and see if we've missed anything else.
New commits:
7b099f7 | 19979: some small additional changes
|
comment:14 Changed 6 years ago by
Changes look fine.
comment:15 Changed 6 years ago by
- Status changed from needs_review to positive_review
ok. we should be all set here then.
comment:17 Changed 6 years ago by
- Status changed from needs_work to positive_review
comment:18 Changed 6 years ago by
- Branch changed from u/bhutz/t/19979 to 7b099f76e9e6217c03320d0c9d85f8011a1b2202
- Resolution set to fixed
- Status changed from positive_review to closed
comment:19 Changed 6 years ago by
- Commit 7b099f76e9e6217c03320d0c9d85f8011a1b2202 deleted
What do you prefer: "Rebecca Lauren Miller" or "Rebecca Miller"?
comment:20 Changed 6 years ago by
Given that comment:6:ticket:20067 was done by rlmiller while this one was by bhutz, I would guess the "Lauren" version.
Last 10 new commits:
Trac 17785: fix segmentation fault for subs
#19880 update openssl to 1.0.2e
Updated Sage version to 7.0.rc1
Deprecate composite_field()
Move sage.rings.arith to sage.arith.misc
Improve location change error message
Raise an error if Sage has been relocated
Updated Sage version to 7.0
19979 Fixing in style in projective products
19979 Fixing style of Product Projective folder