New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prompt manager #507
Prompt manager #507
Conversation
Thanks, this looks good. This should be part of the general move removing all prompt/separator/ui-specific code from the InteractiveShell object to frontends, which definitely won't make it into 0.11. |
Rebased to bring it up to date. |
Can you hook up the qtconsole prompts to your new PromptManager as part of this? I think making that change will help us make the right decisions on the PromptManager, since it will require support for things like HTML prompts without ANSI colors, divorcing the prompt from the InteractiveShell object, and executing things like |
Does it make sense to simply generate an HTML prompt in the kernel, and send it over? Or, more generally, have the frontend make a prompt_request with a template (which could be in any format), and the kernel replies with the filled in version? |
The kernel should probably know exactly nothing about prompts. |
Pragmatically, though, I suspect it will, at least to the extent of filling in slots in a template. |
Possibly, but hopefully not. Any such information should be accessible via the user_expressions/user_variables fields of execute requests. |
OK, that makes more sense, I didn't know about those fields. So the frontend needs to parse the format string, work out which variables it needs, and retrieve them from the backend. I think we still need to think a bit more about how this works, though, because a lot of the details that could go in the prompt aren't accessible from the user namespace without importing extra modules (like |
Yes, the idea was to have frontends request whatever information/expressions they want along with each new prompt (possibly including calls to things like os.getcwd() or time.time(), etc). Then they can build whichever display they want with this. In principle, the same kernel could be monitored by two clients that offer different levels of customizability for the prompts. So the only thing the kernel supplies always is the execution counter, plus whatever variables or expressions may be requested in this form. |
Thomas, this overall looks pretty good, and even if it doesn't get us all the way in terms of restructuring the prompt system as we'd like to, it's probably worth merging now. The prompt rework will likely take a few passes until we have a full separation of duties between kernels (who just handle the namespace) and frontends (who present data from that namespace to the users in various ways, including possibly in a prompt). But there's one thing we should think about: prompts were the one thing for which we kept using Itpl, because it allows the interpolation of arbitrary python expressions, while the new-style string formatting only allows simple variable name access, nothing else. And I know there are projects out there embedding ipython and making weird prompts that update dynamically with function calls, something like: "${some_function()[somearg]}> ". So it seems your change removes that ability, right? Now, I'm not totally against that, as long as we plan to complete the transition into a system that allows users to achieve their goal again, even if by different means (i.e. by asking the kernel for these things and using them to build the prompt). |
@fperez the EvalFormatter I used in the parallel templating code is a step towards exactly that. It doesn't support more than super-simple evals, but that shouldn't be hard. It's in utils.text. For more arbitrary execution, you can do: class EvalFormatter2(Formatter):
def get_field(self, name, args, kwargs):
v = eval(name, kwargs)
return v, name
fmt = EvalFormatter2()
import os
ns = dict(os=os)
fmt.format(' we are in {os.getcwdu()}...', **ns)
# returns 'we are in /home/you/...' |
Plain string formatting can handle |
Note that neither my EvalFormatter in utils.text nor the one I typed up in 5 minutes above are finished implementations (at least something breaks in both), but it shouldn't be hard to get something that works. |
I guess one question is: do we really need to write a new one? I mean, Itpl may be old code, but it works and hasn't needed major updating. So what is the argument for effectively reimplementing itpl? |
Well, just that we can do essentially the same thing with a fraction of the code. Also, Itpl parses the string character-by-character in Python code, so we should get better performance from string formatting. I had the idea of dropping some of our external dependencies, and there are only a couple of places that we actually use Itpl. But I'm not going to push the issue. |
I don't think Itpl handles unicode at all. So either we need to reimplement Itpl with real string formatting (I doubt such a formatter would be more than ~20 lines), or we need to fix Itpl for unicode support. |
Fair enough, I'm ok with both of these arguments. I just wanted to see them :) So Thomas, do you want to finish up a full EvalFormatter and we can merge this puppy? |
ps - we're on IRC now if you want to have a look at any of this right away. |
I could do with hitting the sack now. @minrk, what breaks in the current implementation of EvalFormatter? And is it a condition that we should care about? The only one I'm currently aware of is that you can't readily put a dict literal ( |
A few quick tests of EvalFormatter2 show that it probably supports everything we need for this PR: In [2]: from string import Formatter class EvalFormatter2(Formatter): def get_field(self, name, args, kwargs): v = eval(name, kwargs) return v, name fmt = EvalFormatter2() import os ns = dict(os=os) print fmt.format(' we are in {os.getcwdu()}...', **ns) we are in /home/fperez/ipython/notebooks... In [3]: print fmt.format(' we are in {os.getcwdu()+"hi"}...', **ns) we are in /home/fperez/ipython/notebookshi... In [5]: print fmt.format(' we are in {os.getcwdu()}, a is {a} and a0 is {a[0]}...', **ns) we are in /home/fperez/ipython/notebooks, a is [1, 2] and a0 is 1... So @takluyver, it seems to me that if you update the code to use this (perhaps providing EvalFormatter2 as utils.text.FulEvalFormatter), we'd be good to go. |
Lots of stuff (function calls) don't work in the EvalFormatter in master. EvalFormatter2 seems to work for just about everything, but slicing definitely doesn't work: Like I said above, I didn't spend more than 5-10 minutes on either of these, and I had never looked at any Formatter code before. So perhaps a more careful eye (and a test suite for various known Itpl use cases) would be prudent. |
OK, the issue is that ':' is part of the format specification, see http://www.python.org/dev/peps/pep-3101/. Specifically: Format Specifiers Each field can also specify an optional set of 'format specifiers' which can be used to adjust the format of that field. Format specifiers follow the field name, with a colon (':') character separating the two: "My name is {0:8}".format('Fred') so the parser splits on ':' and returns the stuff after the colon as the format specifier. Which means that this syntax simply is incompatible with arbitrary Python expressions. That doesn't mean we can't use it, just that we'd be setting some restrictions on what kinds of things can go in there. So Thomas, if you can do a little audit of our uses of Itpl and see if a more restrictive version based on EvalFormatter2 (hopefully with a nicer name) isn't a problem, then I have no issues with ditching itpl. And I hope we'll find the time to complete the prompt machinery for frontends before 0.12. |
@fperez, I see, so possibly slicing is out, but it's still possible that even the colon-splitting is implemented in a method that we can easily override. If we can take over right at where the Formatter decides what to do with something inside '{...}', then we should be able to just eval that block, and be done. I don't see any reason for the new one to not just replace EvalFormatter. I just called the example I tossed up EvalFormatter2 because it was my second 10-minute attempt at writing a formatter that evals code. There should be no need for more than one of these. |
Turns out it's actually pretty easy. I looked at the I'll do a PR soon, but I'm writing some tests first. For now a preview: class EvalFormatter(Formatter):
"""A String Formatter that allows evaluation of simple expressions.
Any time a format key is not found in the kwargs,
it will be tried as an expression in the kwargs namespace.
This is to be used in templating cases, such as the parallel batch
script templates, where simple arithmetic on arguments is useful.
Examples
--------
In [1]: f = EvalFormatter()
In [2]: f.format('{n/4}', n=8)
Out[2]: '2'
In [3]: f.format('{range(3)}')
Out[3]: '[0, 1, 2]'
In [4]: f.format('{3*2}')
Out[4]: '6'
"""
# should we allow slicing by disabling the format_spec feature?
allow_slicing = True
# copied from Formatter._vformat with minor changes to allow eval
# and replace the format_spec code with
def _vformat(self, format_string, args, kwargs, used_args, recursion_depth):
if recursion_depth < 0:
raise ValueError('Max string recursion exceeded')
result = []
for literal_text, field_name, format_spec, conversion in \
self.parse(format_string):
# output the literal text
if literal_text:
result.append(literal_text)
# if there's a field, output it
if field_name is not None:
# this is some markup, find the object and do
# the formatting
if self.allow_slicing:
# override format spec, to allow slicing:
field_name = ':'.join([field_name, format_spec])
format_spec = ''
# eval the contents of the field for the object
# to be formatted
obj = eval(field_name, kwargs)
# do any conversion on the resulting object
obj = self.convert_field(obj, conversion)
# expand the format spec, if needed
format_spec = self._vformat(format_spec, args, kwargs,
used_args, recursion_depth-1)
# format the object and append to the result
result.append(self.format_field(obj, format_spec))
return ''.join(result) And the difference between this and the base Formatter is tiny. If we disallow slicing, then the EvalFormatter2 above is cleaner and identical to this when allow_slicing=False. [edit: obviously without the print statement I had initially] |
EvalFormatter updated: #716 Just pass the namespace in which you want the code to execute as the kwargs to format. |
OK, I've merged #716, so that @takluyver can update this one cleanly, to use the new EvalFormatter that has full slicing support for these situations. We do need to make one design decision: for these kinds of expressions, do we value more the ability to do slicing or the ability to specify formatting (i.e. using the ':N' syntax for format specification)? I'm leaning towards the latter, both to keep this syntax in sync with normal Python formatting syntax and because fine-tuning formatting seems to be more likely to be useful in producing things like prompts than slicing. And if slicing is needed, people can always hide it behind a little utility function they can write, so it's not like slicing is impossible. But I'd like to get some consensus before we pull the trigger on the final design for this part. |
I'd lean towards having format string enabled as well. If people do want slicing, they don't even need a utility function: |
It is a hard choice. The reason I discovered that slicing didn't work in the EvalFormatter2 above, is the first example I tried was a truncated version of cwd: f.format("...{os.getcwd()[-10:]}", os=os) however you can actually get slicing by using f.format("...{os.getcwd()[slice(-10,None,None)]}", os=os) So you can get slice behavior without colon notation, if slightly inconvenient to type, but you can't get formatting-notation back in any reasonably way if you allow slicing. Further, if we don't want to support the slicing, the three-line EvalFormatter2 implementation above is much simpler, and probably preferable (and functionally identical when |
OK, it seems we're in agreement that give how slice(a,b,c) is an option (if slightly less convenient), that's the way to go. In that case, we should perhaps go with the really simple EvalFormatter2 (renamed) and be done with it. It's still useful to have the full one @minrk wrote, as it gives us a replacement for itpl with the modern python syntax and unicode support, but for this we should probably stick to the easy, comprehensible 3-liner. |
The only other place we're using Itpl is for expanding variables in %magic and !system calls. My gut feeling is that slicing is more useful than format strings there, but on the other hand we may want to keep closer to the standard Python format as people get more used to that. @minrk: Should EvalFormatter2 replace the existing EvalFormatter that we shipped with 0.11? I think the main difference is that the existing EvalFormatter will treat |
…y use the simple one.
I've just rebased. I've done my best to clear .pyc files, and |
Yes, pylabtools was moved to core. But that change was made in master already, so a rebase should pick it up for you; did it not? |
I thought it should, but your messages gave me the impression that I should |
Just to mention: I haven't got time to go through the docs tonight, but hopefully I can get to it tomorrow. |
OK, yes: working correctly is what you should expect. No problem with the docs, thanks for taking care of them.! |
I've updated the docs; let me know if you spot any other bits that need to be updated. Still to do:
|
I've tidied those bits up - I think this is all OK now. |
Is the test suite passing for you? I'm having some very bizarre problems with the test suite right now on my box at work, but they are odd enough that I think it's my problem and nothing with the codebase. What do you get? |
There was one failure in core, a doctest checking the list of |
OK, great. Let's merge this then as-is, and we'll continue mulling the deeper frontend namespace questions later. No point in holding this any further, lots of work has gone into it. Thanks! |
Prompt manager refactoring: use a new `PromptManager` class responsible for handling everything to do with the prompts. The critical part is its `render` method, which assembles the necessary information, then uses the string formatting introduced in Python 2.6 to fill in the prompt template. I've expanded the definition of 'prompts' to include the auto_rewrite prompt (`"------> "` by default). So there are now four prompts: input, continuation, output, and rewrite. This definition of prompts does not include input/output separators. For now, I've left those as attributes of the main InteractiveShell object.
Thanks, Fernando. It's an odd feeling, not having an open pull request against IPython ;-). |
On Wed, Nov 30, 2011 at 2:20 AM, Thomas
Let's keep it that way for a few days, so we can actually release this |
Prompt manager refactoring: use a new `PromptManager` class responsible for handling everything to do with the prompts. The critical part is its `render` method, which assembles the necessary information, then uses the string formatting introduced in Python 2.6 to fill in the prompt template. I've expanded the definition of 'prompts' to include the auto_rewrite prompt (`"------> "` by default). So there are now four prompts: input, continuation, output, and rewrite. This definition of prompts does not include input/output separators. For now, I've left those as attributes of the main InteractiveShell object.
I'm not expecting this to land before 0.11, but I thought I'd get it on the radar.
Every time I've wondered how prompts were produced, I've ended up rather confused. So this is an attempt at cleaning it up, with a new
PromptManager
class responsible for handling everything to do with the prompts. The critical part is itsrender
method, which assembles the necessary information, then uses the string formatting introduced in Python 2.6 to fill in the prompt template.I've expanded the definition of 'prompts' to include the auto_rewrite prompt (
"------> "
by default). So there are now four prompts: input, continuation, output, and rewrite.This definition of prompts does not include input/output separators. For now, I've left those as attributes of the main InteractiveShell object.