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?
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.
