jimbaker / tagstr

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

Comparison to PEP 501 #7

Closed gvanrossum closed 1 year ago

gvanrossum commented 2 years ago

PEP 501 introduces i-strings, which are syntactically the same as f-strings (using i instead of f as prefix) but return an object representing the entire i-string. The interpolations and their fields can be extracted from this.

About the logging module: Not lambdafying interpolation expressions may avoid some issues with turning plain variables into cells (though I'm not sure such issues exist), but means that there's a minimal cost when i-strings are used as logging arguments -- they are always evaluated (though not formatted or concatenated with the fixed parts). Then again, this is also the case for the current best practice -- passing those expressions as extra arguments to the logging function. However, PEP 501 notes that the logging module supports using various builtin attributes of log records (e.g. %(name)s substitutes the log record name) and suggests that additional integration would be needed to request these (e.g. {'record.name'} would interpolate the log record name). PEP 501 doesn't specify exactly how this should be done, but suggests that the logging module would have to look at the raw text of the interpolated expression to distinguish this usage from interpolating a string variable whose contents happens to be 'record.name'. At that point I'd much rather just have lambdafied interpolations. (UPDATE:) Sadly, using {name} to refer to the record name would be a user trap, since this would override any local variable whose name happened to be a log record attribute (there are over 20 of those).

jimbaker commented 2 years ago

Implemented support for i-strings and the corresponding InterpolationTemplate in examples: https://github.com/jimbaker/tagstr/blob/main/examples/interpolation_template.py

I believe this is a complete implementation as specified by that PEP, other than showing the optional logging integration, which would require writing a minimal expression parser that would work with any dynamically scoped variables (and thus showing the nice aspect of lambdafied expressions).

gvanrossum commented 2 years ago

So the point for logging would be to detect when an interpolation references one of the predefined attributes, right? Delayed interpolation (also mentioned in that section) is not available in the status wou and people seem to be fine with that.

jimbaker commented 2 years ago

Regular interpolation support is available in the implemented example, through the use of InterpolationTemplate's support for two callback parameters in the render method, which has this signature, per PEP 501: def render(self, *, render_template=''.join, render_field=format):

So for a logging specific renderer, we need to write these two functions roughly as follows:

gvanrossum commented 2 years ago

So, to take a specific example, suppose we had a logging template log(i"Too spammy: {pathname}") then in order to recognize 'pathname' as a reference to a LogRecord attribute the logging module would have to look at the parsed_template attribute of the InterpolationTemplate and discover that the raw interpolation text is "pathname" -- or, as you say, it would have to parse that interpolation text into an expression and note mentions of pathname in the leaves of the AST. That's reasonable, and even if we had a dedicated log"Too spammy: {pathname}" tag string we would still have to do the same thing. (Sorry, I just had to research how all this actually works for myself and write it up here, even if it looks like you already know all that.)

I'm not sure why you'd want to wrap the field -- in the current implementation fields aren't treated special AFAIK -- you'd write log("Too spammy: %(pathname)s").

(Oh, I'm writing log(...) but it would have to be logging.debug(...), logging.warn(...) etc. So with the dedicated tag string it'd become logging.warn(log"Too spammy: {pathname}"). Maybe the tag shouldn't exactly be log.)

gvanrossum commented 1 year ago

There's now a Discourse thread about reopening PEP 501. I'm not excited about that, but some folks on Discourse appear to be.

pauleveritt commented 1 year ago

I believe we can close this. The author of the revived work said he'd adopt this tagstr work.

jimbaker commented 1 year ago

I believe we can close this. The author of the revived work said he'd adopt this tagstr work.

Good to heaar about this collaboration! Closing accordingly.