search

eigenhombre / PyClojure

(Parts of) Clojure implemented on top of Python
Other
113 stars 13 forks source link

Added float and python function support #4

Closed lclarkmichalek closed 12 years ago

lclarkmichalek commented 12 years ago

This pull request should probabaly be split up into several smaller ones, but the merge I had to do was a bit ugly, so it's probabaly easier if they all go in together.

Added float support. This is almost identical to the int support.

Added python calling support. When a list is evaled, if the first item is an atom, it is looked up in the scope. If the val is a function, then it is called, with the other items in the list being the arguments. A GlobalScope special scope has all python default functions in it, along with a selection of choice operators.

eigenhombre commented 12 years ago

Hi Laurie,

Couple of comments.

  1. unit tests for floats? can you add some?
  2. I think the regex for floats should probably be: r'(([0-9]+(.[0-9])?[eE][+-]?[0-9]+)|(.[0-9]+([eE][+-]?[0-9]+)?)|([0-9]+.[0-9]))' (this is from my compiler class -- getting this one right is actually rather tricky)
  3. the python function calling looks pretty cool; thanks also for making the lists into immutable tuples.
  4. this last remark is the trickiest one. I am wondering why you made lisplexer and lispparser into classes. I realize it is somewhat more pythonic, but Rich Hickey's talk on Simple Made Easy and clojure and functional programming in general are making me more suspicious of the idiom of using classes everywhere, even when they are not needed. The current lexer and parser exist essentially as closures; they have no state or methods other than the PLY internals. So, I'm curious why you chose to change from a functional idiom to classes for these? Is there some direction you want to take this part of the code which will require classes?

Many thanks, John

lclarkmichalek commented 12 years ago

The main reason I changed the lisplexer and lispparser into classes was because having them as functions completely confused me. I had no idea how ply was getting access to the namespace it was being called in (still don't quite understand how it's doing this), and it seemed too much like magic. Explicitly passing the object that has the lexing/parsing rules feels better to me than having ply magically yank them out of the namespace in which it was called. There's no great technical reason, but it confused me a lot when I first saw it.

However, on a more broader note, I think functions vs classes will be an issue in the codebase. In core.py there is the tostring function; if I had written that module, each type would have a tostring method. I think however, it will be easier in the long run to use the functional idiom.

On the float regex; I just googled "float regex" and yanked the first result, thinking exponential form would be easier to implement as a separate rule. However, I'll use that regex instead, thanks.

And yes, I'll add some unittests for float support and function calling.

lclarkmichalek commented 12 years ago

All tests now pass

eigenhombre commented 12 years ago

Cool! I have a question or two about your latest commits but will have to review later as today is quite full. Cheers!

On Feb 15, 2012, at 1:05 PM, Laurie Clark-Michalek wrote:

All tests now pass

Reply to this email directly or view it on GitHub: https://github.com/eigenhombre/PyClojure/pull/4#issuecomment-3986699

eigenhombre commented 12 years ago

Merging commits as is - I will probably add a few more unit tests for the floats... understood about the confusion regarding the lexer/parser magic -- the PLY module is unfortunately somewhat magical. If you think it's clearer expressed as a class, let's go with that for now.