Skip to content
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

Autobalance #14081

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Autobalance #14081

wants to merge 8 commits into from

Conversation

jb-leger
Copy link

This PR adds a mode for automatic opening of brackets, braces and braces.

This follows a strange idea that appeared on Saturday the 13th, following the other strange idea in #14077.

The logic used is as follows:

  • if the mode is enabled
  • and if the cell is only one line long
  • and if there are unmatched [{()}]
  • and if it is enough to add opening symbols at the beginning and/or closing symbols at the end

then the line is modified.

Technical points:

  • I chose not to use prefilters, but input transformers for this, because prefilters work with the check/handle logic, which does not allow to have at the same time the addition of symbols and the use of a magic (like %time for example).
  • the TransformerManager didn't have a member allowing to come back to the shell, and it's essential for me to access the shell config and to be able to display the modified line (when there is a modification)
  • the transformations had no way to have additional arguments and only received the lines. I had to add kwargs to be able to add arguments that transformations are free to use or not.

Demo:

Python 3.11.2 (main, Mar 13 2023, 12:18:29) [GCC 12.2.0]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.14.0.dev -- An enhanced Interactive Python. Type '?' for help.

In [1]: 1+1)/2
  Cell In[1], line 1
    1+1)/2
       ^
SyntaxError: unmatched ')'


In [2]: %autobalance
Autobalance enabled

In [3]: 1+1)/2
------> (1+1)/2
Out[3]: 1.0

In [4]: 1+1)/(3+4
------> (1+1)/(3+4)
Out[4]: 0.2857142857142857

In [5]: i+1)/2 for i in range(5)]+[1,2
------> [(i+1)/2 for i in range(5)]+[1,2]
Out[5]: [0.5, 1.0, 1.5, 2.0, 2.5, 1, 2]

In [6]: my_values = i+1)/2 for i in range(5)]+[1,2
------> my_values = [(i+1)/2 for i in range(5)]+[1,2]

In [7]: %config TerminalInteractiveShell.show_rewritten_input = False

In [8]: 1+1)/(3+4
Out[8]: 0.2857142857142857

This is my first time diving into IPython code (and contributing to it), I may have missed a lot things. I place this PR in draft for two reasons:

  • the changes made to the TransformerManager logic (with shell passing and kwargs on transformations) seem a bit intrusive to me, and discussing them seems like the best option.
  • I have not yet written the specific tests for this new behavior.

Finally, I think it is important to mention the adapted xkcd.


# only for type checking, do not import this, it lead to a circular
# import
if False:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do

from typing import TYPE_CHECKING

...

if TYPE_CHECKING:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 10f3017.

symbols are added in the begin of the expression, and closings symbols
at the end of the line. E.g. `1+2)/2]*(2+3` becomes `[(1+2)/2]*(2+3)`.
""",
).tag(config=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines 244 to 245
shell = kwargs.get("shell")
display = kwargs.get("display", False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you are adding kwargs everywhere to get shell and display here, but that will break API of external transforms that don't take a kwargs. So we may wan to have a

class Autobalancer:

    def __init__(self, shell):
        ....

    def __call__(...):



Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good idea, thank you. This solution is a lot less intrusive. Done in 79a23bf.

@@ -579,6 +658,7 @@ def __init__(self):
]
self.line_transforms = [
cell_magic,
autobalance,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here you should be able to have Autobalancer(shell)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Continuation of last thread. Done in 79a23bf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants