Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh the file free_quadratic_module #34625

Closed
fchapoton opened this issue Oct 2, 2022 · 19 comments
Closed

Refresh the file free_quadratic_module #34625

fchapoton opened this issue Oct 2, 2022 · 19 comments

Comments

@fchapoton
Copy link
Contributor

Cleanup, doc formatting, remove commented code, etc.

CC: @tscrim @kwankyu @kliem @slel

Component: quadratic forms

Author: Frédéric Chapoton

Branch/Commit: 561f669

Reviewer: Matthias Koeppe, Kwankyu Lee

Issue created by migration from https://trac.sagemath.org/ticket/34625

@fchapoton fchapoton added this to the sage-9.8 milestone Oct 2, 2022
@fchapoton
Copy link
Contributor Author

New commits:

0a42b28refresh the file free_quadratic_module

@fchapoton
Copy link
Contributor Author

Commit: 0a42b28

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/34625

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2022

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2022

comment:2
     EXAMPLES:
 
     Since this is an embedded vector subspace with echelonized basis,
-    the echelon_coordinates() and user coordinates() agree::
+    the :meth:`echelon_coordinates` and :meth:`user coordinates` agree::
 

This does not look like a valid method name.

Otherwise LGTM

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

5d42e9edetail in free_quadratic_module

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2022

Changed commit from 0a42b28 to 5d42e9e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2022

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

156914arefresh the file free_quadratic_module

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 3, 2022

Changed commit from 5d42e9e to 156914a

@fchapoton
Copy link
Contributor Author

comment:5

ok, thanks, fixed now (I squashed the commits)

@fchapoton
Copy link
Contributor Author

comment:6

green lights, so please review

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 5, 2022

Changed branch from u/chapoton/34625 to u/klee/34625

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 5, 2022

Changed commit from 156914a to 561f669

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 5, 2022

comment:8

Positive review modulo my edits


New commits:

561f669More edits

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 5, 2022

Changed reviewer from Matthias Koeppe to Matthias Koeppe, Kwankyu Lee

@kwankyu

This comment has been minimized.

@kwankyu kwankyu changed the title refresh the file free_quadratic_module Refresh the file free_quadratic_module Oct 5, 2022
@fchapoton
Copy link
Contributor Author

comment:10

thanks, lights are green, so I am setting to positive now

@kwankyu
Copy link
Collaborator

kwankyu commented Oct 5, 2022

comment:11

Thanks.

@vbraun
Copy link
Member

vbraun commented Oct 16, 2022

Changed branch from u/klee/34625 to 561f669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants