PR: Load history from IPython directly #7141

impact27 · 2018-05-17T14:33:59Z

Pull Request Checklist

Read and followed this repo's Contributing Guidelines
Based your PR on the latest version of the correct branch (master or 3.x)
Followed PEP8 for code style
Ensured your pull request hasn't eliminated unrelated blank lines/spaces,
modified the spyder/defaults directory, or added new icons/assets
Wrote at least one-line docstrings for any new functions
Added at least one unit test covering the changes, if at all possible
Described your changes and the motivation for them below
Noted what issue(s) this pull request resolves, creating one if needed
Included a screenshot, if this PR makes any visible changes to the UI

Description of Changes

Spyder now loads the history from iPython instead of duplicating the history in a local file.
That is less confusing for the user as the history in iPython and the history in historylog are now the same.

Issue(s) Resolved

Fixes #7142

pep8speaks · 2018-05-17T14:34:04Z

Hello @impact27! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file spyder/app/tests/test_mainwindow.py:

Line 1795:1: E302 expected 2 blank lines, found 1
Line 1807:80: E501 line too long (86 > 79 characters)
Line 1809:5: E265 block comment should start with '# '
Line 1815:80: E501 line too long (86 > 79 characters)
Line 1820:80: E501 line too long (88 > 79 characters)
Line 1821:22: W291 trailing whitespace
Line 1822:80: E501 line too long (84 > 79 characters)

In the file spyder/plugins/history/plugin.py:

Line 188:9: E265 block comment should start with '# '
Line 232:9: E265 block comment should start with '# '

In the file spyder/plugins/history/tests/test_plugin.py:

Line 136:1: W293 blank line contains whitespace

Comment last updated at 2019-06-02 10:59:50 UTC

ccordoba12 · 2018-05-17T19:55:30Z

@impact27, looks good at first glance, but we really need a test for this because it's a major internal change. You need to add it to test_mainwindow.py (because it involves two plugins), and it could be very simple: evaluate a command in the console and register that the last entry in the History Log corresponds to that command.

Also, please create a new issue for this because #7080 is really solved by your other PR.

bcolsen · 2018-05-17T22:06:17Z

I did some tests on this and it works. I think this is the way forward. Having the history stored per entered commands in this way could lead to great new functionality in the history explorer.

The only regression is the old history had date headers, but they weren't very useful. Here's an example of the date headers

plt.figure("capacity2", clear=True)
plt.plot(cycles.cycle,cycles.c_cap)

## ---(Thu May 17 15:26:14 2018)---
x
import sympy
sympy.__version__

ccordoba12 · 2018-05-17T23:36:32Z

@impact27, please also migrate this to the master branch.

impact27 · 2018-05-18T06:55:13Z

I opened #7142

impact27 · 2018-05-18T06:58:16Z

The dates are saved in the ipython database so It could be added back.

CAM-Gerlach · 2018-05-18T17:13:41Z

Thanks @impact27 . FYI you should rebase your original work on master, not merge it, so as to not dirty the history. You can get the original back with reflog in case you don't have it already in a branch, or an interactive rebase -i.

Also, apparently AppVeyor on Py3 isn't able to find pytest-mock to install it and is thus failing; not sure why. @ccordoba12 ?

CAM-Gerlach · 2018-05-18T18:10:18Z

@pep8speaks suggest diff for testing purposes

bcolsen · 2018-05-19T05:56:02Z

I've been testing this and it's working great.

Summary of text below: I think it would be best to have one history tab per console.

On a more broader view it seems like there is a lot of code here that seems to be used to get all the ipython consoles to talk to one history tab so it mimics the way the previous code worked.

From a use case prospective I think this isn't a good idea to have the history mixed and it would be better and simpler to have IPython take care of the history entirely. We can just take what the console gives not worry about it...it works great. This would require us to have one history tab per console, but luckily the history widget already has tabs implemented.

Currently, If you change between tabs and run commands in each the history gives you a mess of code that is most likely unassociated and you can tell what commands were executed in each console. The up arrow history at the ipython prompt keeps the history separate and when both sessions are closed the two histories are appended into the main history one console after the other so the code is still in blocks that are still associated.

The hardest part would be to synchronize the tab switching between the consoles and the history.

I would open a new issue but this idea would greatly impact this work.

ccordoba12 · 2018-05-20T00:43:11Z

@impact27, as @CAM-Gerlach suggested, please rebase this PR in top of latest master. Your merge was really unnecessary.

ccordoba12 · 2018-05-20T00:43:23Z

I've been testing this and it's working great.

Great!

Currently, If you change between tabs and run commands in each the history gives you a mess of code that is most likely unassociated and you can tell what commands were executed in each console.

What if we use a QStackedWidget instead to only display the history of the current console, but change to another history when users select a different console? That's what we do in the Variable Explorer and I think it works quite well.

I would open a new issue but this idea would greatly impact this work.

Please do. We can work on it after this PR is merged.

CAM-Gerlach · 2018-05-20T00:49:36Z

What if we use a QStackedWidget instead to only display the history of the current console, but change to another history when users select a different console? That's what we do in the Variable Explorer and I think it works quite well.

The only disadvantage there, that isn't the case for the variable explorer, is if a user wants to view or copy one console's history while working in another. It could be solved by a "lock" toggle locking the history shown to that of the currently selected console (like the one for Help). However, we'd then need to implement tabs in the history log like we have in the console to navigate between different console's logs independently, so not sure if the different use cases that require it would be worth the effort at this point.

ccordoba12 · 2018-05-20T02:15:30Z

@CAM-Gerlach, we can still use a QStackedWidget widget and an option that allows users to select the console's history they want to see at the moment (but that would be changed to the current console as soon as they change consoles).

I really don't want to use tabs for this because I really think it makes no sense. In any case, please stop discussing that in this PR (because it's unrelated to @impact27's work) and open a new issue about it.

CAM-Gerlach · 2018-05-20T02:32:14Z

Sorry 😮

@bcolsen was the one with the detailed proposal above and said he could create the new issue, so I'll await that unless he wants me to do it—I don't want to get in his way there.

ccordoba12 · 2018-05-20T15:41:39Z

Sorry

No worries, I just didn't want us to keep polluting this PR ;-)

impact27 · 2018-05-21T15:15:38Z

I rebased the pull request

ccordoba12 · 2018-05-21T17:44:39Z

spyder/app/mainwindow.py

@@ -1261,7 +1261,9 @@ def post_visible_setup(self):

        # Show history file if no console is visible
        if not self.ipyconsole.isvisible:
-            self.historylog.add_history(get_conf_path('history.py'))
+            self.historylog.add_history(
+                    "iPython history",


Let's call this simply History.

Also, its a moot point now but just for the record, its IPython—they are actually pretty emphatic on that distinction.

ccordoba12 · 2018-05-21T20:39:26Z

spyder/plugins/console.py

-        Not used anymore since v2.0"""
-        historylog.add_history(self.shell.history_filename)
-        self.shell.append_to_history.connect(historylog.append_to_history)
-


This needs to stay. It's used to save and load history in the internal console. So please put it back it.

ccordoba12 · 2018-05-21T20:40:04Z

spyder/plugins/history.py

    #------ Public API ---------------------------------------------------------
-    def add_history(self, filename):
+    def add_history(self, filename, history_list):


history_list must be optional, so that history in the internal console can still work.

ccordoba12 · 2018-05-21T20:40:56Z

spyder/plugins/ipythonconsole.py

-            client.append_to_history.connect(self.historylog.append_to_history)
-
+            self.historylog.add_history(
+                    "iPython history",


Please change this to History

ccordoba12 · 2018-05-21T20:43:15Z

@impact27, this looks good, but you need to preserve saving history in the internal console.

CAM-Gerlach · 2018-09-04T06:17:21Z

@impact27 Looks like this got hit big-time by the split-plugins merge. Are you interested in finishing it?

With regard to your question, I'm not really sure why there need be a tab in the History log for Internal Console history, considering it is only used for testing and introspecting Spyder itself rather than production work, and can in any case still be accessed inside the internal console's one pane anyway, but even so it looks like it still works just fine.

impact27 · 2018-09-05T10:18:36Z

Unfortunately I don't really have time these days. Does somebody else want to finish that?

ccordoba12 · 2018-09-05T20:34:21Z

I'll finish it. I'll implement the QStackedWidget solution I proposed some time ago.

CAM-Gerlach · 2019-05-06T04:16:02Z

@ccordoba12 @impact27 Hey, is this ever going to be finished? A lot of conflicting files have built up over the time this has been sitting around...

goanpeca · 2019-05-06T15:40:52Z

@ccordoba12 thoughts on this? Do you want some of the other devs to tackle this at some point?

ccordoba12 · 2019-05-06T17:25:07Z

We'll take another look after we release beta2.

ccordoba12 · 2019-05-22T10:57:38Z

@impact27, could you update and finish this PR? For now let's go with my proposed approach, i.e. showing the history of the current console when switching from one to another.

impact27 · 2019-06-02T11:02:49Z

I rebase it on the master and it still works, but I am not sure how to do the QStackedWidget, so I would be glad if someone else could finish it.

ccordoba12 · 2019-06-04T11:05:56Z

I rebase it on the master and it still works

Thanks a lot for that!

but I am not sure how to do the QStackedWidget

Ok, no problem. We'll help you with that part then.

impact27 · 2020-09-25T06:20:06Z

Closing because too many conflicts accumulated and this would need a rewrite

ccordoba12 changed the title ~~Load history from ipython directly~~ PR: Load history from ipython directly May 17, 2018

ccordoba12 added this to the v4.0beta1 milestone May 17, 2018

impact27 changed the base branch from 3.x to master May 18, 2018 07:04

bcolsen mentioned this pull request May 20, 2018

Have the History widget display a separate history for each console #7155

Open

ccordoba12 changed the title ~~PR: Load history from ipython directly~~ PR: Load history from IPython directly May 20, 2018

impact27 force-pushed the fix_history branch from f0e4b7e to 4762d7b Compare May 21, 2018 13:42

CAM-Gerlach assigned impact27 May 21, 2018

CAM-Gerlach added component:History Log type:Techdebt labels May 21, 2018

ccordoba12 reviewed May 21, 2018

View reviewed changes

bcolsen mentioned this pull request May 31, 2018

Add options to not include runfile, execute a cell and run selsection in the history #7233

Open

ccordoba12 modified the milestones: v4.0beta1, v4.0beta2 Aug 8, 2018

ccordoba12 modified the milestones: v4.0beta2, v4.0beta3 Dec 2, 2018

impact27 force-pushed the fix_history branch from 87fad33 to c4ab28a Compare June 2, 2019 10:55

fix history

ea32208

impact27 force-pushed the fix_history branch from c4ab28a to ea32208 Compare June 2, 2019 10:59

ccordoba12 modified the milestones: v4.0beta3, v4.0beta4 Jun 17, 2019

oscargus mentioned this pull request Jul 19, 2019

PR: Add possibility to disable history logging #9858

Closed

3 tasks

ccordoba12 modified the milestones: v4.0beta4, v4.0beta5 Jul 20, 2019

ccordoba12 modified the milestones: v4.0beta5, future Aug 10, 2019

impact27 mentioned this pull request Aug 13, 2019

PR: Add permanent history to the debugger #10010

Merged

3 tasks

goanpeca added the v:future label Feb 23, 2020

goanpeca removed this from the future milestone Feb 23, 2020

impact27 closed this Sep 25, 2020

ccordoba12 removed the v:future label Sep 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Load history from IPython directly #7141

PR: Load history from IPython directly #7141

impact27 commented May 17, 2018 •

edited

Loading

pep8speaks commented May 17, 2018 •

edited

Loading

ccordoba12 commented May 17, 2018

bcolsen commented May 17, 2018 •

edited

Loading

ccordoba12 commented May 17, 2018

impact27 commented May 18, 2018

impact27 commented May 18, 2018

CAM-Gerlach commented May 18, 2018 •

edited

Loading

CAM-Gerlach commented May 18, 2018

bcolsen commented May 19, 2018

ccordoba12 commented May 20, 2018

ccordoba12 commented May 20, 2018

CAM-Gerlach commented May 20, 2018

ccordoba12 commented May 20, 2018 •

edited

Loading

CAM-Gerlach commented May 20, 2018 •

edited

Loading

ccordoba12 commented May 20, 2018 •

edited

Loading

impact27 commented May 21, 2018

ccordoba12 May 21, 2018

CAM-Gerlach May 21, 2018

ccordoba12 May 21, 2018

ccordoba12 May 21, 2018

ccordoba12 May 21, 2018 •

edited

Loading

ccordoba12 commented May 21, 2018

CAM-Gerlach commented Sep 4, 2018

impact27 commented Sep 5, 2018

ccordoba12 commented Sep 5, 2018

CAM-Gerlach commented May 6, 2019

goanpeca commented May 6, 2019

ccordoba12 commented May 6, 2019

ccordoba12 commented May 22, 2019

impact27 commented Jun 2, 2019

ccordoba12 commented Jun 4, 2019

impact27 commented Sep 25, 2020

PR: Load history from IPython directly #7141

PR: Load history from IPython directly #7141

Conversation

impact27 commented May 17, 2018 • edited Loading

Pull Request Checklist

Description of Changes

Issue(s) Resolved

pep8speaks commented May 17, 2018 • edited Loading

Comment last updated at 2019-06-02 10:59:50 UTC

ccordoba12 commented May 17, 2018

bcolsen commented May 17, 2018 • edited Loading

ccordoba12 commented May 17, 2018

impact27 commented May 18, 2018

impact27 commented May 18, 2018

CAM-Gerlach commented May 18, 2018 • edited Loading

CAM-Gerlach commented May 18, 2018

bcolsen commented May 19, 2018

ccordoba12 commented May 20, 2018

ccordoba12 commented May 20, 2018

CAM-Gerlach commented May 20, 2018

ccordoba12 commented May 20, 2018 • edited Loading

CAM-Gerlach commented May 20, 2018 • edited Loading

ccordoba12 commented May 20, 2018 • edited Loading

impact27 commented May 21, 2018

ccordoba12 May 21, 2018

Choose a reason for hiding this comment

CAM-Gerlach May 21, 2018

Choose a reason for hiding this comment

ccordoba12 May 21, 2018

Choose a reason for hiding this comment

ccordoba12 May 21, 2018

Choose a reason for hiding this comment

ccordoba12 May 21, 2018 • edited Loading

Choose a reason for hiding this comment

ccordoba12 commented May 21, 2018

CAM-Gerlach commented Sep 4, 2018

impact27 commented Sep 5, 2018

ccordoba12 commented Sep 5, 2018

CAM-Gerlach commented May 6, 2019

goanpeca commented May 6, 2019

ccordoba12 commented May 6, 2019

ccordoba12 commented May 22, 2019

impact27 commented Jun 2, 2019

ccordoba12 commented Jun 4, 2019

impact27 commented Sep 25, 2020

impact27 commented May 17, 2018 •

edited

Loading

pep8speaks commented May 17, 2018 •

edited

Loading

bcolsen commented May 17, 2018 •

edited

Loading

CAM-Gerlach commented May 18, 2018 •

edited

Loading

ccordoba12 commented May 20, 2018 •

edited

Loading

CAM-Gerlach commented May 20, 2018 •

edited

Loading

ccordoba12 commented May 20, 2018 •

edited

Loading

ccordoba12 May 21, 2018 •

edited

Loading