On Sun, Apr 18, 2010 at 7:14 PM, Aaron S. Meurer <asmeu...@gmail.com> wrote:
> OK, It's mostly little things now.
> - Reindent your __repr__ function.  I don't even know why that works.
> - Maybe David or others will have an opinion on this.  Should it _repr_ as
> this?


Yes, it doesn't appear that __repr__ conforms to Python standards
http://docs.python.org/reference/datamodel.html
__str__ is still missing.


> In [15]: PrimeField(5)
> Out[15]: [0, 1, 2, 3, 4]
> It can become unwieldily for large primes.  Also, I would at least expect
> {}, to denote that it is a set (and not make it look like a list).
> - The first lines of your examples need to be reindented .
> - It is no longer in the main __init__.py file.
> - I would like to see __contains__, but it can be done later (it isn't
> blocking it).
> - You have trailing whitespace at the end of the file.
> Aaron Meurer
> On Apr 18, 2010, at 1:46 PM, Kasun Samarasinghe wrote:
>
> hi,
> while I m working with github, can you check this patch. I addressed all the
> comments.
> thank you,
> kasun
>
> On Sat, Apr 17, 2010 at 10:42 PM, Aaron S. Meurer <asmeu...@gmail.com>
> wrote:
>>
>> This patch is not applying for me.  Can you squash all of your patches
>> into one commit (use git rebase -i master)?  Also, things would be easier if
>> you pushed up to a github repository.
>> Now that it works, I can comment on the actual code.
>> In [2]: a = PrimeField(5)
>> In [4]: a.elements()
>> Out[4]: [0, 1, 2, 3, 4]
>> In [5]: a.multiplicative_inverse(4) # I think this should return 4, not
>> -1.
>> Out[5]: -1
>> In [7]: 4 in a # This ought to work (__contains__)
>>
>> ---------------------------------------------------------------------------
>> TypeError                                 Traceback (most recent call
>> last)
>> /Users/aaronmeurer/Documents/python/sympy/sympy/<ipython console> in
>> <module>()
>> TypeError: argument of type 'instance' is not iterable
>> In [9]: a = PrimeField(4)
>> ValueError: Invalid Argument
>> Use more descriptive messages than "Invalid Argument" for your errors.
>>  For example, for this one, you might use "Argument of PrimeField must be a
>> prime number (received 4)", or something similar.
>> In [22]: a
>> Out[22]: <sympy.abstractalgebra.finitefield.PrimeField instance at
>> 0x175ca08>
>> Like David said, please include some kind of __str__ or __repr__ method so
>> this prints nicer.
>> In your doctests, and elsewhere, use one space around an =, such as "gf7 =
>> PrimeField(7)".
>> I think this is almost ready.
>> Aaron Meurer
>> On Apr 16, 2010, at 4:29 PM, Kasun Samarasinghe wrote:
>>
>> hi aaron,
>> i managed to fix the doctest problem, here is the patch for that part
>> only.
>> kasun
>> On Sat, Apr 17, 2010 at 12:12 AM, Kasun Samarasinghe
>> <kwsamarasin...@gmail.com> wrote:
>>>
>>> I have fixed the nested problem and white spaces problem. In my machine
>>> it passes the tests.
>>> here __init__.patch is the import statement in sympy/__init__.py
>>> thank you,
>>> kasun
>>>
>>> On Fri, Apr 16, 2010 at 11:27 PM, Kasun Samarasinghe
>>> <kwsamarasin...@gmail.com> wrote:
>>>>
>>>> also can you please have a look at my doctests, since it fails.
>>>> thanks
>>>>
>>>> On Fri, Apr 16, 2010 at 11:20 PM, Kasun Samarasinghe
>>>> <kwsamarasin...@gmail.com> wrote:
>>>>>
>>>>> hi aaron
>>>>> will it convert the tab into four spaces if I run the strip utility?
>>>>> thanks
>>>>> kasun
>>>>>
>>>>>
>>>>>
>>>>> On Fri, Apr 16, 2010 at 10:20 PM, Aaron S. Meurer <asmeu...@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> Sorry, it still doesn't work for me.  The problem is that you have
>>>>>> abstractalgebra nested twice.  Also, I think you might need to add 
>>>>>> something
>>>>>> to the main sympy/__init__.py (assuming we want this imported with from
>>>>>> sympy import *; do we?).
>>>>>> The ./bin/strip_whitespace utility will help with the other failure.
>>>>>>  Setup your text editor to use 4 spaces instead of tabs:
>>>>>>
>>>>>> ________________________________________________________________________________
>>>>>>
>>>>>>  
>>>>>> /users/aaronmeurer/documents/python/sympy/sympy/sympy/abstractalgebra/abstractalgebra/test_primefield.py
>>>>>>   File
>>>>>> "/users/aaronmeurer/documents/python/sympy/sympy/sympy/abstractalgebra/abstractalgebra/test_primefield.py",
>>>>>> line 1, in <module>
>>>>>>     from sympy.abstractalgebra.finitefield import PrimeField
>>>>>> ImportError: No module named abstractalgebra.finitefield
>>>>>>
>>>>>> ________________________________________________________________________________
>>>>>> __
>>>>>> sympy/utilities/tests/test_code_quality.py:test_whitespace_and_exceptions
>>>>>> ___
>>>>>>   File
>>>>>> "/users/aaronmeurer/documents/python/sympy/sympy/sympy/utilities/tests/test_code_quality.py",
>>>>>> line 97, in test_whitespace_and_exceptions
>>>>>>     check_directory_tree(SYMPY_PATH, test, exclude)
>>>>>>   File
>>>>>> "/users/aaronmeurer/documents/python/sympy/sympy/sympy/utilities/tests/test_code_quality.py",
>>>>>> line 58, in check_directory_tree
>>>>>>     file_check(fname)
>>>>>>   File
>>>>>> "/users/aaronmeurer/documents/python/sympy/sympy/sympy/utilities/tests/test_code_quality.py",
>>>>>> line 82, in test
>>>>>>     assert False, message_tabs % (fname, idx+1)
>>>>>> AssertionError: File contains tabs instead of spaces:
>>>>>> /users/aaronmeurer/documents/python/sympy/sympy/sympy/abstractalgebra/abstractalgebra/finitefield.py,
>>>>>> line 11.
>>>>>> Aaron Meurer
>>>>>> On Apr 15, 2010, at 5:49 PM, Kasun Samarasinghe wrote:
>>>>>>
>>>>>> hi aaron,
>>>>>> I managed to make it passed test, please give me the comment. attached
>>>>>> the patch with this
>>>>>> thank you
>>>>>> kasun
>>>>>>
>>>>>> On Fri, Apr 16, 2010 at 12:31 AM, Ronan Lamy <ronan.l...@gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Le jeudi 15 avril 2010 à 13:46 -0600, Aaron S. Meurer a écrit :
>>>>>>> > - I think PrimeField should subclass from Expr or Basic (though I
>>>>>>> > could be wrong on this one).
>>>>>>> >
>>>>>>> No, it should not. Instances of PrimeField are equivalent to classes
>>>>>>> like Integer or Rational. I think sympy is not quite ready for this
>>>>>>> yet.
>>>>>>> In the current model, PrimeField "should" be a metaclass and can only
>>>>>>> subclass BasicType (which is empty). It is its instances which
>>>>>>> "should"
>>>>>>> be subclasses of Basic. And yes, this would probably be very messy.
>>>>>>>
>>>>>>> So, it is reasonable to implement finite fields outside the main
>>>>>>> hierarchy (note that polynomials are also outside the main hierarchy,
>>>>>>> ultimately for the same reason). When sympy grows ways to manipulate
>>>>>>> types, they can be brought back into the fold.
>>>>>>>
>>>>>>> > - How is this different from the GF
>>>>>>> >  implementation in polys?  Should this rather just be providing a
>>>>>>> > user
>>>>>>> >  interface to that?
>>>>>>>
>>>>>>> I think it's the opposite: polys should interface with the generic
>>>>>>> implementation. Ultimately, the implementations should be merged, but
>>>>>>> the code should move out of polys and into the new module.
>>>>>>>
>>>>>>> Ronan
>>>>>>>
>>>>>>> --
>>>>>>> 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-Finite-Field-Implementation-Prime-Field-Only.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.
>> <doctestfix.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-Prime-Field-Implementation.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.

Reply via email to