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

PR: Add option to use Jedi in the IPython console + warning on greedy completer #6832

Merged
merged 6 commits into from Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions spyder/config/main.py
Expand Up @@ -158,6 +158,7 @@
'startup/use_run_file': False,
'startup/run_file': '',
'greedy_completer': False,
'ipy_jedi_completer': False,
Copy link
Member

Choose a reason for hiding this comment

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

Please change this to jedi_completer. We already know that it's an IPython console option because it's in the right section.

'autocall': 0,
'symbolic_math': False,
'in_prompt': '',
Expand Down
33 changes: 30 additions & 3 deletions spyder/plugins/ipythonconsole.py
Expand Up @@ -477,15 +477,42 @@ def setup_page(self):
run_file_group.setLayout(run_file_layout)

# ---- Advanced settings ----
# Enable Jedi completion in Ipyhton Console
Copy link
Member

Choose a reason for hiding this comment

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

"IPython" is the correct spelling and capitalization, though ipython also works.

ipy_jedi_group = QGroupBox(_("Jedi completion in Ipython console"))
Copy link
Member

Choose a reason for hiding this comment

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

IPython Console, to follow the above convention. Also, needs the article "the" in front of it. So:

"Jedi Completion in the IPython Console"

However, as this is a section title, for consistancy with the general style and with the others on the page, it should be shortened to:

"Jedi Completion"

or something similar, and in Title Case.

ipy_jedi_label = QLabel(_("Enable Jedi based <tt>Tab</tt> completion in"
Copy link
Member

Choose a reason for hiding this comment

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

"Jedi-based", also needs a space at the end.

"the Ipython console. E.g. completion of"
Copy link
Member

Choose a reason for hiding this comment

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

"the IPython Console; works with e.g. nested lists, etc.

Copy link
Member

Choose a reason for hiding this comment

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

Alternately, you can cut everything after the semicolon and instead add "similar to the greedy completer, but without evaluating the code." A little longer and more visually awkward than cutting the second line, but cleaner flow on the textual side.

"nested lits etc. This is similar to the "
Copy link
Member

Choose a reason for hiding this comment

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

I'd lean toward cutting this sentence, as it saves a full line, and including it in the popup tip instead; better IMO to put that extra descriptive text in the popup tip as its designed for then clutter the UI with every last detail.

"greedy compiler, but without evaluating the code<br>"
Copy link
Member

Choose a reason for hiding this comment

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

If you do include this, the name of the below option is the greedy completer, not "compiler".

"<b>Attention:</b> This can slow down your console "
Copy link
Member

Choose a reason for hiding this comment

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

More concise and idiomatic: Warning: Slows down the console when using large dataframes!

"interaction when working with large dataframes!"))
ipy_jedi_label.setWordWrap(True)
ipy_jedi_box = newcb(_("Use Jedi Completer in Ipython"), "ipy_jedi_completer",
Copy link
Member

Choose a reason for hiding this comment

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

"Use Jedi completion in the IPython Console"

tip="<b>Warning</b>: This can slow down your console"
Copy link
Member

Choose a reason for hiding this comment

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

Fix alignment; should be a few spaces right to align one col right of the opening paren of newcb. Also, doesn't this need to be translated too, (with _) or do we not translate our tips?

Copy link
Member

Choose a reason for hiding this comment

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

Also, use the above wording.

"when working with large dataframes ")

ipy_jedi_layout = QVBoxLayout()
ipy_jedi_layout.addWidget(ipy_jedi_label)
ipy_jedi_layout.addWidget(ipy_jedi_box)
ipy_jedi_group.setLayout(ipy_jedi_layout)

# Greedy completer group
greedy_group = QGroupBox(_("Greedy completion"))
greedy_group = QGroupBox(_("Greedy completion in Ipython console"))
Copy link
Member

Choose a reason for hiding this comment

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

To be consistent with the other headers on this page, it should just read "Greedy Completion".

greedy_label = QLabel(_("Enable <tt>Tab</tt> completion on elements "
"of lists, results of function calls, etc, "
"<i>without</i> assigning them to a "
"variable.<br>"
Copy link
Member

Choose a reason for hiding this comment

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

Should probably eliminate this <br>, it looks awkward and takes up an extra line especially with the additional warning statement.

"For example, you can get completions on "
Copy link
Member

Choose a reason for hiding this comment

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

Since you're adding so much extra text, I'd recommend you simplify this to something like:

"Get completions on e.g. [...] and [...]"

"things like <tt>li[0].&lt;Tab&gt;</tt> or "
"<tt>ins.meth().&lt;Tab&gt;</tt>"))
"<tt>ins.meth().&lt;Tab&gt;</tt> <br><br>"
"<b>ATTENTION:<br> Ipython's Greedy completer has "
Copy link
Member

Choose a reason for hiding this comment

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

"IPython's greedy completer has "

Or better yet, rewrite the whole thing to read as follows:

"Due to a bug, IPython's greedy completer requires a leading space for some completions; e.g. np.sin(<Space>np.<Tab>) works, while np.sin(np.<Tab>) doesn't.

"a bug</b> that requires a leading "
"<tt>&lt;Space&gt;</tt> "
"for some completions to work. e.g. chane "
Copy link
Member

Choose a reason for hiding this comment

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

Typo; I presume you mean "change", and e.g. needs to be capitalized if starting a sentence, though I recommend rewriting it as above.

"<tt>np.sin(np.&lt;Tab&gt;</tt> "
"to <tt>np.sin(&lt;Space&gt;np.&lt;Tab&gt;</tt> "
", and completion for string filenames has "
Copy link
Member

Choose a reason for hiding this comment

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

Everything after the comma on this line is redundant, you used the same phrase above and it is directly implied by what you've stated. I know it relates to your specific problem, but including every possible case (and this is one few if any others have complained about) is an issue for the documentation IMO, not the UI.

"to start with a space to work e.g.: "
"' filena<tt>&lt;Tab&gt;</tt> "))
greedy_label.setWordWrap(True)
greedy_box = newcb(_("Use the greedy completer"), "greedy_completer",
tip="<b>Warning</b>: It can be unsafe because the "
Expand Down Expand Up @@ -575,7 +602,7 @@ def setup_page(self):
_("Graphics"))
tabs.addTab(self.create_tab(run_lines_group, run_file_group),
_("Startup"))
tabs.addTab(self.create_tab(greedy_group, autocall_group, sympy_group,
tabs.addTab(self.create_tab(ipy_jedi_group, greedy_group , autocall_group, sympy_group,
Copy link
Member

Choose a reason for hiding this comment

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

Remove extra space added after greedy_group.

prompts_group), _("Advanced Settings"))

vlayout = QVBoxLayout()
Expand Down
1 change: 1 addition & 0 deletions spyder/utils/ipython/kernelspec.py
Expand Up @@ -124,6 +124,7 @@ def env(self):
'SPY_RUN_FILE_O': CONF.get('ipython_console', 'startup/run_file'),
'SPY_AUTOCALL_O': CONF.get('ipython_console', 'autocall'),
'SPY_GREEDY_O': CONF.get('ipython_console', 'greedy_completer'),
'SPY_IPY_JEDI_O': CONF.get('ipython_console', 'ipy_jedi_completer'),
Copy link
Member

Choose a reason for hiding this comment

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

Again, remove IPY_ and ipy_ here.

'SPY_SYMPY_O': CONF.get('ipython_console', 'symbolic_math'),
'SPY_RUN_CYTHON': self.is_cython
}
Expand Down
4 changes: 3 additions & 1 deletion spyder/utils/ipython/start_kernel.py
Expand Up @@ -69,10 +69,12 @@ def kernel_config():
# Until we implement Issue 1052
spy_cfg.InteractiveShell.xmode = 'Plain'

# Jedi completer in iypthon console
ipy_jedi_o = os.environ.get('SPY_IPY_JEDI_O') == 'True'
Copy link
Member

Choose a reason for hiding this comment

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

Remove ipy_ and IPY_ also here.

# - Using Jedi slow completions a lot for objects with big repr's.
# - Jedi completions are not available in Python 2.
if not PY2:
spy_cfg.IPCompleter.use_jedi = False
spy_cfg.IPCompleter.use_jedi = ipy_jedi_o

# Run lines of code at startup
run_lines_o = os.environ.get('SPY_RUN_LINES_O')
Expand Down