jimbaker / tagstr

This repo contains an issue tracker, examples, and early work related to PEP 999: Tag Strings
51 stars 6 forks source link

Small nits & comments on the PEP #44

Closed lysnikolaou closed 2 months ago

lysnikolaou commented 3 months ago

Comments:

pauleveritt commented 3 months ago

@lysnikolaou First, thanks a bunch for taking the time to review.

  1. Tag function names. I think it's just to point out that you could use a built-in callable, and the example of f shows that any callable with a name that's in the namespace is usable. I'm ok with dropping it.

  2. Performance. We can expand on the text a bit. But we can't really measure without an implementation, and we shouldn't focus on implementation until the PEP is underway. Maybe we just add some text that says "of course something not grabbing frames will be faster"? Or perhaps, just a little speed test comparing frames vs. not-frames, outside of tag strings?

  3. Format spec. Do you mean discuss eager eval in the "Format Specification"? Or cover it more in "Interpolation" section, where eager is briefly mentioned? (This "Interpolation" section comes before "Format Specification".) Or do you mean, before both of those?

  4. Equal sign. Good idea, I will get something added to the "Specification" section.

  5. Return non-string. Do you mean return string-like things? Or do you mean, opening the door to returning anything? Can you outline your thinking on what controversy to head off?

lysnikolaou commented 3 months ago
  1. and the example of f shows that any callable with a name that's in the namespace is usable

My understanding is that current string prefixes cannot be used as tag names. Hence an f-string will continue to be an f-string even if there's an f function defined in the current namespace. Am I missing something?

2. Maybe we just add some text that says "of course something not grabbing frames will be faster"?

While that may be useful, I don't think it adds much value. It should be relatively clear and both PEP 498 and PEP 701 do not include a Performance section, since the feature is very strictly not about performance but about ease of use.

3. Do you mean discuss eager eval in the "Format Specification"? Or cover it more in "Interpolation" section, where eager is briefly mentioned?

Oh that's right. I missed that mention in the Interpolation section, which AFAIU is part of the Format specification. No action here then.

5. Return non-string. Do you mean return string-like things? Or do you mean, opening the door to returning anything? Can you outline your thinking on what controversy to head off?

AFAIU the door is already open to returning pretty much anything. The tag function is specifed as def __call__(self, *args: Decoded | Interpolation) -> Any: which means it can return anything. The only two remaining references to this is the len'hello' example and the fact that the example tag returns just the args which is a tuple.

I guess my point is more like a question. Is this something we consider to be a star feature (to me it's something that makes tag strings incredibly powerful) or is it a side-effect of the specification and something we've not thought about a lot? If it's the former, then a bigger section that's dedicated to that seems fitting, explaining the rationale and giving examples on how it can be used. If it's the latter, then we need to have a bigger discussion on why we're allowing this.

jimbaker commented 3 months ago
  1. and the example of f shows that any callable with a name that's in the namespace is usable

My understanding is that current string prefixes cannot be used as tag names. Hence an f-string will continue to be an f-string even if there's an f function defined in the current namespace. Am I missing something?

Agreed, the discussion about people don't seem to confuse usage here is itself confusing. Let's remove.

  1. Maybe we just add some text that says "of course something not grabbing frames will be faster"?

While that may be useful, I don't think it adds much value. It should be relatively clear and both PEP 498 and PEP 701 do not include a Performance section, since the feature is very strictly not about performance but about ease of use.

+1

  1. Return non-string. Do you mean return string-like things? Or do you mean, opening the door to returning anything? Can you outline your thinking on what controversy to head off?

AFAIU the door is already open to returning pretty much anything. The tag function is specifed as def __call__(self, *args: Decoded | Interpolation) -> Any: which means it can return anything. The only two remaining references to this is the len'hello' example and the fact that the example tag returns just the args which is a tuple.

I guess my point is more like a question. Is this something we consider to be a star feature (to me it's something that makes tag strings incredibly powerful) or is it a side-effect of the specification and something we've not thought about a lot? If it's the former, then a bigger section that's dedicated to that seems fitting, explaining the rationale and giving examples on how it can be used. If it's the latter, then we need to have a bigger discussion on why we're allowing this.

Definitely it's the former, returning any object is a very important feature of tag strings, and is also what tagged template literals in JS support.

We need some more motivation here.

jimbaker commented 3 months ago

Definitely it's the former, returning any object is a very important feature of tag strings, and is also what tagged template literals in JS support.

Following up further. Let's say I have an html tag function, used like so:

x = html"<div>Hello, {name}</div>"

with the type signature of

def html(*args: Decoded | Interpolation) -> HTML:
    ...

Then a developer could implement this HTML class such that could support a number of different usages (possibly all of them):

  1. Marker class - recursively compose HTML out of other HTML objects, and this is safe because of this marking.
  2. DOM usage - one could ask about attributes like x.tagname, x.attrs, and x.children. Of course this can be lazy, to support a large DOM.
  3. __str__, which then let's us do print(x) - but the interpolation of name ensures that it's HTML escaped.
  4. __iter__, to support WSGI usage. In particular this iteration can evaluate interpolations in the order they are actually being written out. Recursive HTML objects implies using yield from.
  5. ASGI, and so on, depending on how intprepid our developer is. 😁

Similar observations could be applied for a developer writing a sql tag to support Django's QuerySet or SQLAlchemy's Query, and so forth.

The one thing we see in these examples - including for Django and SQLAlchemy - is that the build object doesn't actually do anything until asked, and that should be a recommended best practice (possibly in the PEP).

I think we can and should discuss HTML object building like above in the PEP, although not an actual implementation. That could be done in a tutorial, for example, although that would not be a formal part of the PEP.

jimbaker commented 3 months ago

I have a nit myself. There's this paragraph:

The expressions that are extracted from the string are evaluated in the context where the tag string appeared. This means the expression has full access to its lexical scope, including local and global variables. Any valid Python expression can be used, including function and method calls.

The way it's displayed suggests that it is a quote from PEP 498. But the only reference I could find to the verbatim quote with a Google search is this PEP. Formatting error?

pauleveritt commented 3 months ago

We have some actionable points. How to proceed? Merge these nits and I open a new PR with suggested changes?

lysnikolaou commented 2 months ago

Thanks for the explanation @jimbaker! That's all what I had in mind as well.

I think we can and should discuss HTML object building like above in the PEP, although not an actual implementation.

Agreed with this. Not sure if it should be HTML specifically, but an example where a tag string is used to build a class is certainly worth adding. This would make things much clearer.

lysnikolaou commented 2 months ago

We have some actionable points. How to proceed? Merge these nits and I open a new PR with suggested changes?

I'd say let's merge this and create issues for the remaining actions so that we can assign them between ourselves to split the effort. I'll also get started with porting the implementation to main soon.

pauleveritt commented 2 months ago

Very good, I'll extract all the points, then squash-and-merge.

pauleveritt commented 2 months ago

I've extracted the notes from here and from a chat with Jim. Closing, on to next step.