#17030: Knot Theory as a part of GSoC 2014.
-------------------------------------+-------------------------------------
Reporter: amitjamadagni | Owner: amitjamadagni
Type: task | Status: needs_work
Priority: major | Milestone: sage-6.4
Component: algebraic | Resolution:
topology | Merged in:
Keywords: | Reviewers: Miguel Marco
Authors: Amit Jamadagni, | Work issues:
Miguel Marco | Commit:
Report Upstream: N/A | e6e17c401b8b879cd041dafc120d3943b3f2dacb
Branch: | Stopgaps:
u/amitjamadagni/knots |
Dependencies: |
-------------------------------------+-------------------------------------
Changes (by {'newvalue': u'Amit Jamadagni, Miguel Marco', 'oldvalue':
u'amitjamadagni, mmarco'}):
* cc: tscrim (added)
* reviewer: mmarco => Miguel Marco
* milestone: => sage-6.4
* author: amitjamadagni, mmarco => Amit Jamadagni, Miguel Marco
Comment:
It looks like a bad resolution to a merge, but it's a simple enough fix.
Also on a quick look through:
- Doc formatting should follow:
{{{
INPUT:
- ``name`` -- some description that is very long and
note the alignment
OUTPUT:
- these are not usually complete sentences but short descriptions
}}}
- Move the nice documentation from the `__init__` into the class-level doc
so people can actually see it.
- All methods must have doctests.
- It's my pythonic to use `isinstance(input_, list)` instead of
`type(input_) == list`.
- `input_` is a bad variable name (according to python standards), just
use `input` (or perhaps the slightly more descriptive `data`)
- Don't raise `Exception`, instead raise `ValueError` or `TypeError`
(after killing with fire and holy water :P).
- Change
{{{#!python
pd_error = _pd_check_(input_)
if pd_error == True:
pass
elif pd_error == False:
pass
}}}
into (the pythonic)
{{{#!python
if _pd_check_(input_):
pass
else:
pass
}}}
- Use `is not None` instead of `!= None`.
- Make `_dt_internal_` a proper method of the class.
- Remove the trailing underscore of `_pd_check_`, ,`_dt_internal_` etc. as
methods with a leading and trailing underscore are usually Sage special
functions (and naming conventions of python).
- Is "dowker" a proper name (and hence, should be capitalized in the
documentation)?
- The method name `Seifert_Matrix` should be `seifert_matrix` (naming
conventions again).
- Could you change the name of `ncomponents` to `number_of_components`
(although you can keep it as a shorthand alias)?
- This needs to be added to the documentation (probably as a new
component).
I'll make a more detailed pass through the code after these are addressed.
I also don't know how much of the math I'll be able to review as I've
studied a little knot theory, but possibly not enough to check everything.
Although Miguel could count as the reviewer on that front.
I think this will be a great addition to Sage once it's been polished.
Best,[[BR]]
Travis
--
Ticket URL: <http://trac.sagemath.org/ticket/17030#comment:12>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
