Issue 16: objects with indices (tensors)
http://code.google.com/p/sympy/issues/detail?id=16

Comment #19 by andy.terrel:
I haven't reviewed anything here yet, but I'll give it a shot.  Just so you  
know my tensor calculus is fuzzy so if I
say something silly let me know, I mostly have some comments about Sympy  
code and style.  I only had time to
get through the first patch I will look at the next one a bit later.

As far as the repo goes, I believe Ondrej is saying to make a repo (such as  
at github.org) and then we can pull
from it, as explained at  
http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#public-
repositories ,

AFAIK, we are suppose to use PEP 8 (  
http://www.python.org/dev/peps/pep-0008/ ) for python style.  With
that your code has quite a few lines over 80 characters and trailing  
whitespaces.  The trailing whitespace
actually break the tests.




For /sympy/indexed/indexed.py

Indexed and Indexed_Components overload __init__ instead of __new__.  This  
is out of sync with the rest of the
code and also causes errors for objects. For example:

Indexed('a',[k],[m,n]) ==  Indexed('b',[k],[m,n])

causes some errors.

Also you are putting dimensions and components in the keywords, why not  
make these explicit and pass
on the assumptions to the basic class handler?  Right now you get them  
leaked to the assumptions:

In [13]: g = Indexed('g', [i, j], components = ((r**2, 1, 1), (0, t**3, 1)))
In [19]: g.assumptions0
Out[19]:
{'components': ((r**2 , 1, 1), (0, t**3 , 1))}

Also it would be nice to document how the components are suppose to work.   
For the above example is it that
g_{r**2,1} = 1 or g_{1,1} = r**2?  Maybe there is more room for what is  
going on here or perhaps a reference
for the mathematically challenged (like me).

I also wonder if we should derive the Indexed from Basic instead of Atom,  
since it can be broken down into
other parts.  Perhaps a design similar to the Function class would be more  
appropriate.


Sympy are suppose to be immutable, so setting new attributes should return  
new versions of the class.  For
example:

     def set_components(self, components):
         ...
         self.components = components

needs to be rethought.  I'm actually not sure what is going on here for  
example why would you do this instead
of create a new object.

I'm not sure why this function is needed, just calling components should be  
sufficient:

     def get_components(self):
         return self.components

Could you add an examples to free_indices and einstein_sum?  In all the  
other functions it really helps.

Other than that I it looks good to me, but I need to go review some of the  
rules of tensors to check if things
are correct algorithmically.  This looks really cool and I would love to  
have it in.





-- 
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"sympy-issues" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/sympy-issues?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to