Hi,

I've been looking into the fcode and codegen implementation. Sorry for the delay. First of all: I'm really happy with the work being done. I wish I could dedicate more time to the reviewing.

I have a few small patches, but consider them as suggestions. (See attachments.)


0001-just-call-fcode-and-derivation-friendly-settings.patch

I'm more in favor of calling fcode directly. Then there is no need to write set_some_setting methods in the printers, which is much cleaner API-wise. The overhaed should be small, but I don't know what the actual reason was to have a self._printer atribute in FCodeGen? I also added a test to make sure that the settings are handled in an inheritance-friendly way.


0002-Fixed-missing-comma-in-__all__-in-codegen.py.patch

This seems futile, but it is probably something that happens easily. Is it possible to add a code quality test that checks of 'from mod import *' works for every module?


0003-Fixed-return-value-assignment-in-FCodeGen.patch

Just a small bug fix.


Some more suggestions that came to mind:

- Rename FCodeGen to F95CodeGen (also use .f95 then). In the long run it would be interesting to have an F77CodeGen too. I guess it will be different enough to justify two classes.

- The expr argument of the Routine constructor was one of the next things I wanted to modify before I stopped working on codegen. Now the constructor tries to figure out which expression is a return variable, and so on. It would be more convenient to replace expr by results, i.e. a list of ResultBase instances. This gives some more freedom to the user and it makes the implementation way easier. The current trick with the Equality class is rather counterintuitive.

cheers,

Toon

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

Attachment: patches.tar.gz
Description: GNU Zip compressed data

Reply via email to