Thanks for taking the time to review.
Stefan Behnel wrote:
> General comment: I'm not sure templating is better than creating the tree by
> hand. For example, you use node.body below, which can expand to anything.
>
> Since I consider writing transformers a rare thing, maybe it would be better
> to really write down the node creation code and stick in the bits from the
> original tree. To help doing that, we could have a tree dumper that writes out
> the class creation calls, so that you can write the Cython code, dump the
> tree, and just modify the tree creation code to fit your needs. I find that
> simpler and safer than your parsing approach.
(I saw your other comment so I'll disregard the "safer" bit.)
This is the kind of question I don't think there's much point discussing
in length:
- It doesn't affect stability/chance of introducing bugs (except, I'd
argue, for an advantage in code readability for the string notation...)
- It doesn't affect any interfaces or APIs or anything anywhere
- It can be switched to another approach in 20 minutes at any time
without affecting anything else. And TreeFragment can actually take a
manually constructed "template" node tree as a constructor argument too!
So you wouldn't need to change anything but replace the string itself.
It's like discussing whether one should use a for loop or while loop! We
can't always discuss things at this level IMO. My stance on this is
simply that if you implement the with statement, you can create the
nodes however you want, and if I write it, I'm free to use strings and
TreeFragment. What do you think?
(Like I said, the reason I wrote TreeFragment is to have easy unit
testing. But once it's there, I'd use it for the with statement too myself.)
>> - Some changes to Transform.py which I hope goes through... there's a
>> Visitor object there; using the "process_ClassName" pattern (I think
>> that was the conclusion for future performance reasons).
>
> Did we really reach a conclusion on this?
Well, we had two in favor of vtable-friendliness (and you and me
against); so I considered that if I switched my vote, it was 3/1. I
really don't care that much either way, I just wanted to move forward
without getting hung up into trivial details. Bike sheds and so on. (As
you don't think transforms will be that useful, I guess you wouldn't
care that much anyway? I think they will be critical though...and better
safe than sorry, i.e. better vtable-friendly than not.)
>> A SourceDescriptor can currently be a FileSourceDescriptor, in which
>> case things work like before (it gives the filename on __str__ so much
>> code needed not change),
>
> Please make that a method on the SourceDescriptor, like "get_filename()".
> Calling str() on it reads like you wanted to print the source.
The real problem here was my email message.
A Source***Descriptor*** is an abstraction for the filename, not its
contents, so str() does not read like you want to print the source.
str() is called when you want to inform a human user (in an error
message) about which source the code you're displaying comes from. It
could say "disk file: /home/..." rather than just "/home/...", but I
didn't want to change behaviour for files.
"get_description()" would be ok if str() is considered too ambigious though.
In the cases where an actual filename is needed, I did (at least I hope
I did) always insert something to the effect of:
if not isinstance(source_desc, FileSourceDescritor): raise
AssertionError("Expected a file disk source here")
do_something(source_desc.filename)
(For instance, Main.Context does this a lot, but Main.Context also
always pass in a FileSourceDescriptor to begin with. The Context that
the string parser uses instead implements methods in a way so that this
assumption about using file sources is changed.)
--
Dag Sverre
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev
