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.

Reply via email to