OK.  

- You need to fix the commit message. It now reads "changed patch" instead of 
"Adding Implementation of Finite Fields (Prime Fields)".  It also wouldn't hurt 
to add more information to it.
- You misunderstood what I meant for docstrings. You want
"""
Summary of function (one line).

More info about function (as many lines as necessary).

**Examples**
>>> example usage of your function (these are the doctests)
"""
See other modules in SymPy for examples of this.

- I still get the error from below, as well as this one:
AssertionError: File does not end with a newline: 
/users/aaronmeurer/documents/python/sympy/sympy/sympy/abstractalgebra/finitefield.py,
 line 70

Please run ./bin/test and./bin/doctest and make sure all tests pass before 
committing.

- What about putting things in __init__.py, and subclassing (second to last 2 
points below)? Right now, I can't really test the code because I can't import 
it.
- What about GF(p**n), where p is prime and n is an integer?  Does this require 
more work to implement than just GF(p) (or is it just forthcoming)?

Aaron Meurer
On Apr 15, 2010, at 2:39 PM, Kasun Samarasinghe wrote:

> I changed according to the comments. Please have a look,
> 
> kasun
> 
> On Thu, Apr 15, 2010 at 9:55 PM, Kasun Samarasinghe 
> <kwsamarasin...@gmail.com> wrote:
> hi,
> 
> this is just the prime field implementation, which just provides the prime 
> field arithmetic and operations. There is no relationship to polynomials 
> unless we define galois polynomials using this prime field. In my proposal I 
> suggested a separate finite field implementation and this is the first part 
> of it. 
> 
> thank you,
> kasun
> 
> 
> On Thu, Apr 15, 2010 at 9:46 PM, Aaron S. Meurer <asmeu...@gmail.com> wrote:
> Hi.  Some things:
> 
> - Use lower case for file names.
> - I get some test failures:
> 
>  File 
> "/users/aaronmeurer/documents/python/sympy/sympy/sympy/abstractalgebra/test/test_primefield.py",
>  line 1, in <module>
>    from sympy.abstractalgebra.FiniteField import PrimeField
> ImportError: No module named abstractalgebra.FiniteField
> 
> and also
> 
> AssertionError: File contains generic exception: 
> /users/aaronmeurer/documents/python/sympy/sympy/sympy/abstractalgebra/FiniteField.py,
>  line 13
> 
> which brings me to my next point….
> - Don't use generic exceptions.  Use ValueError or TypeError or whatever is a 
> relevant exception.
> - For the docstrings, make the first line a summary of the method or 
> function, then have a blank line followed by anything else.
> - Please add doctests to the new methods
> - You need to add things to __init__.py so that they can be used.
> - I think PrimeField should subclass from Expr or Basic (though I could be 
> wrong on this one).
> 
> and finally, my main point:
> - How is this different from the GF implementation in polys?  Should this 
> rather just be providing a user interface to that?
> 
> Aaron Meurer
> 
> On Apr 15, 2010, at 12:34 PM, Kasun Samarasinghe wrote:
> 
> > Hi,
> >
> > I tried to implement Prime  Fields which is the first part of my GSOC 
> > project. I attached the patch with
> > this. Please review it and comment.
> >
> > Thnak you,
> > kasun
> >
> > --
> > You received this message because you are subscribed to the Google Groups 
> > "sympy" group.
> > To post to this group, send email to sy...@googlegroups.com.
> > To unsubscribe from this group, send email to 
> > sympy+unsubscr...@googlegroups.com.
> > For more options, visit this group at 
> > http://groups.google.com/group/sympy?hl=en.
> > <0001-Adding-Implementation-of-Finite-Fields-Prime-Fields.patch>
> 
> --
> You received this message because you are subscribed to the Google Groups 
> "sympy" group.
> To post to this group, send email to sy...@googlegroups.com.
> To unsubscribe from this group, send email to 
> sympy+unsubscr...@googlegroups.com.
> For more options, visit this group at 
> http://groups.google.com/group/sympy?hl=en.
> 
> 
> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "sympy" group.
> To post to this group, send email to sy...@googlegroups.com.
> To unsubscribe from this group, send email to 
> sympy+unsubscr...@googlegroups.com.
> For more options, visit this group at 
> http://groups.google.com/group/sympy?hl=en.
> <0001-changed-patch.patch>

-- 
You received this message because you are subscribed to the Google Groups 
"sympy" group.
To post to this group, send email to sy...@googlegroups.com.
To unsubscribe from this group, send email to 
sympy+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/sympy?hl=en.

Reply via email to