PR: Remember format for floats in DataFrame editor #3611

jitseniesen · 2016-10-28T14:41:58Z

This is #3539 ~~raised from the dead~~ but based now against 3.x branch.

This pull request is to make Spyder remember the format that the user sets for floats in the DataFrameEditor by storing it in the user configuration settings.

A new config setting dataframe_format is introduced in the variable explorer section in the user config. This setting contains the format, without the leading % character (e.g., if the format is '%.3f' then the setting is stored as '.3f'). The reason for this is that it is apparently impossible to have a config setting with a % character, because this is used in ConfigParser for interpolation.
This setting is passed (after adding a % character in the front) from the VariableExplorer plugin via the NamespaceBrowser and CollectionEditor widgets to the DataFrameEditor widget, where it is used to set the format for floats.
If the user changes the format in the DataFrameEditor widget, then this information is passed back using a sig_option_changed signal via CollectionEditor and NamespaceBrowser to VariableExplorer. There the leading % character is stripped and the format is stored in the user config.

jitseniesen · 2016-10-28T14:43:36Z

I addressed @goanpeca's comments, which were mostly about formatting but he did point out one important issue: if the user gives a format that starts with a %, an assertion is triggered. This is handled more gracefully by the last commit (d765a74).

goanpeca · 2016-10-28T14:46:01Z

LGTM :-) I would say marge, but we are waiting on @ccordoba12 3.02 release :-)

ccordoba12 · 2016-11-30T15:42:08Z

spyder/plugins/variableexplorer.py

        for name in REMOTE_SETTINGS:
-            settings[name] = CONF.get(VariableExplorer.CONF_SECTION, name)
+            settings[name] = CONF.get(section, name)


I don't know why this was written like this before, but now you should be able to use self.get_option here (instead of calling CONF directly)

This is a static method so there is no self. I will convert it to a normal method but I discovered that it is called in a number of places so I need to change them ...

Ok, thanks for looking at this. I think it'd be nicer to have it as a method.

ccordoba12 · 2016-11-30T15:46:13Z

spyder/widgets/variableexplorer/collectionseditor.py

+        At the moment, this signal can only come from a DataFrameEditor.
+        """
+        assert option_name == 'dataframe_format'
+        self.parent().set_dataframe_format(new_value)


Could we move this method to dataframeeditor? It has nothing to do with collectionseditor :-)

The collectionseditor creates the dataframeeditor so it has to inject the relevant configuration options (here only the dataframe format) in the dataframeeditor.

As I understand the widget/plugin separation, widgets should get all relevant configuration options at creation time. Widgets are not supposed to query the corresponding plugin because it should be possible to run them stand alone. In this instance, the collectioneditor needs to keep track of the dataframe format, so I don't see how to move this method to dataframeeditor.

You're right. I tried to come up with a better design but yours is the only viable alternative I can see.

Let's modify this couple of lines to

if option_name == 'dataframe_format': self.parent().set_dataframe_format(new_value)

to not make change_option too restrictive.

ccordoba12 · 2016-11-30T15:49:42Z

This looks very good to me. I just added some comments to simplify the code a little bit ;-)

ccordoba12 · 2016-11-30T23:12:06Z

@malliwi88 this also needs a rebase/merge with 3.x now :-)

The format is stored without a percent sign in the user configuration (e.g., '.3g' instead of '%.3g'), because otherwise the percent sign would interfere with ConfigParser's interpolation feature.

- Change get_settings() in variable explorer plugin to also retreive the dataframe_format setting. - The setting is then passed to the NamespaceBrowser widget by add_shellwidget(). - Add a member variable to NamespaceBrowser for storing the setting. - Extend NamespaceBrowser.setup() to set the setting.

- Add ReadOnlyCollectionsModel member variable to store the config setting, and set it in its initializer. - Change initializer of RemoteCollectionsEditorTableView to accept the config setting and pass it on when constructing a ReadOnlyCollectionsModel. - Change NamespaceBrowser.setup() to pass on the setting when constructing a RemoteCollectionsEditorTableView. - Add a test.

- In CollectionsDelegate.createEditor(), when creating a DataFrameEditor, call set_format with the config setting. - Add a test.

- Add a signal sig_option_changed to DataFrameEditor. - Raise this signal in change_format(). - Add a test.

- When creating a DataFrameEditor in CollectionsDelegate.createEditor(), connect its sig_option_changed signal to the new .change_option() function. - Add CollectionsDelegate.change_option(), to call BaseTableView.set_dataframe_format() on the table view of the collection editor. - Add a test.

- In VariableExplorer.add_shellwidget(), connect sig_option_changed from the newly created NamespaceBrowser to a new .change_option() member function. - Add VariableExplorer.change_option() to check whether the option being changed is the dataframe format, in which case the leading '%' character from the format is stripped. Then sig_option_changed is emitted from the plugin, which leads to the new config setting being saved.

Commit da2aa25 broke the assert.

This hopefully resolves flaky tests on qt4 caused by lingering DataFrameModel.

Add full stops to some comments; remove one out-dated comment; use .format() for formatting.

If user changes the format in the dataframe editor and provides a format which does not start with '%', then (without this commit) the assert in VariableExplorer.change_option() will be triggered.

jitseniesen · 2016-12-01T21:57:44Z

Rebased but I still have to look at the comments re collectioneditor.py

As a consequence. VariableExplorer.get_settings() converts from a static method to a normal method.

jitseniesen · 2016-12-02T11:12:12Z

I replied to the comment re collectioneditor and took care of the rest.

ccordoba12 · 2016-12-04T17:39:19Z

spyder/plugins/variableexplorer.py

        """
+        section = VariableExplorer.CONF_SECTION


This variable is not needed anymore.

ccordoba12 · 2016-12-04T18:16:36Z

I suggested a last couple of changes, then I'd say it's ready to merge :-)

Also remove an unused variable.

jitseniesen · 2016-12-04T21:40:25Z

I suggested a last couple of changes, then I'd say it's ready to merge :-)

Done

ccordoba12 · 2016-12-04T23:24:13Z

Ok, thanks a lot @jitseniesen!

Fixes #2728 Conflicts: - spyder/widgets/variableexplorer/collectionseditor.py - spyder/widgets/variableexplorer/namespacebrowser.py

jitseniesen added this to the v3.1 milestone Oct 28, 2016

goanpeca assigned jitseniesen Oct 31, 2016

jitseniesen changed the base branch from master to 3.x November 26, 2016 18:17

ccordoba12 reviewed Nov 30, 2016

View reviewed changes

jitseniesen added 11 commits December 1, 2016 21:48

Variable explorer: Add config option for dataframe editor's format

c298107

The format is stored without a percent sign in the user configuration (e.g., '.3g' instead of '%.3g'), because otherwise the percent sign would interfere with ConfigParser's interpolation feature.

Variable explorer: Pass dataframe_format config to dataframe editor

b4f6fef

- In CollectionsDelegate.createEditor(), when creating a DataFrameEditor, call set_format with the config setting. - Add a test.

Variable explorer: Emit signal if dataframe editor format is changed

b79f33b

- Add a signal sig_option_changed to DataFrameEditor. - Raise this signal in change_format(). - Add a test.

Variable explorer: Remove assert which is no longer satisfied

54ad6cf

Commit da2aa25 broke the assert.

Variable explorer testing: Register widgets with qtbot

c644f13

This hopefully resolves flaky tests on qt4 caused by lingering DataFrameModel.

Variable explorer: Code formatting

e6bc093

Add full stops to some comments; remove one out-dated comment; use .format() for formatting.

Variable explorer: Check that format provided by user starts with '%'

8d0dc3a

If user changes the format in the dataframe editor and provides a format which does not start with '%', then (without this commit) the assert in VariableExplorer.change_option() will be triggered.

jitseniesen force-pushed the dataframe-format branch from d765a74 to 7df80b9 Compare December 1, 2016 21:56

jitseniesen force-pushed the dataframe-format branch from 7df80b9 to 8d0dc3a Compare December 1, 2016 22:11

Variable explorer: Use get_option() instead of CONF.get()

a14e8e7

As a consequence. VariableExplorer.get_settings() converts from a static method to a normal method.

malliwi88 mentioned this pull request Dec 3, 2016

DataFrame viewer should display tooltips for truncated column headers #3721

Closed

ccordoba12 reviewed Dec 4, 2016

View reviewed changes

spyder/plugins/variableexplorer.py

"""

section = VariableExplorer.CONF_SECTION

Copy link

Member

ccordoba12 Dec 4, 2016

Choose a reason for hiding this comment

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

This variable is not needed anymore.

ccordoba12 changed the title ~~PR: Remember format for floats in dataframe editor~~ PR: Remember format for floats in DataFrame editor Dec 4, 2016

Variable explorer: Make CollectionsDelegate.change_option() more general

fa9cc24

Also remove an unused variable.

ccordoba12 merged commit 73a8971 into spyder-ide:3.x Dec 4, 2016

ccordoba12 added a commit that referenced this pull request Dec 4, 2016

Merge from 3.x: PR #3611

f2b36f0

Fixes #2728 Conflicts: - spyder/widgets/variableexplorer/collectionseditor.py - spyder/widgets/variableexplorer/namespacebrowser.py

jitseniesen deleted the dataframe-format branch September 29, 2017 12:07

jitseniesen mentioned this pull request Sep 11, 2018

Config is resetted after a CONF_VERSION bump #7875

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Remember format for floats in DataFrame editor #3611

PR: Remember format for floats in DataFrame editor #3611

jitseniesen commented Oct 28, 2016 •

edited by ccordoba12

Loading

jitseniesen commented Oct 28, 2016 •

edited

Loading

goanpeca commented Oct 28, 2016

ccordoba12 Nov 30, 2016

jitseniesen Dec 1, 2016

ccordoba12 Dec 1, 2016

ccordoba12 Nov 30, 2016

jitseniesen Dec 2, 2016

ccordoba12 Dec 4, 2016

ccordoba12 commented Nov 30, 2016

ccordoba12 commented Nov 30, 2016 •

edited by goanpeca

Loading

jitseniesen commented Dec 1, 2016

jitseniesen commented Dec 2, 2016

ccordoba12 Dec 4, 2016

ccordoba12 commented Dec 4, 2016 •

edited

Loading

jitseniesen commented Dec 4, 2016

ccordoba12 commented Dec 4, 2016

PR: Remember format for floats in DataFrame editor #3611

PR: Remember format for floats in DataFrame editor #3611

Conversation

jitseniesen commented Oct 28, 2016 • edited by ccordoba12 Loading

jitseniesen commented Oct 28, 2016 • edited Loading

goanpeca commented Oct 28, 2016

ccordoba12 Nov 30, 2016

Choose a reason for hiding this comment

jitseniesen Dec 1, 2016

Choose a reason for hiding this comment

ccordoba12 Dec 1, 2016

Choose a reason for hiding this comment

ccordoba12 Nov 30, 2016

Choose a reason for hiding this comment

jitseniesen Dec 2, 2016

Choose a reason for hiding this comment

ccordoba12 Dec 4, 2016

Choose a reason for hiding this comment

ccordoba12 commented Nov 30, 2016

ccordoba12 commented Nov 30, 2016 • edited by goanpeca Loading

jitseniesen commented Dec 1, 2016

jitseniesen commented Dec 2, 2016

ccordoba12 Dec 4, 2016

Choose a reason for hiding this comment

ccordoba12 commented Dec 4, 2016 • edited Loading

jitseniesen commented Dec 4, 2016

ccordoba12 commented Dec 4, 2016

jitseniesen commented Oct 28, 2016 •

edited by ccordoba12

Loading

jitseniesen commented Oct 28, 2016 •

edited

Loading

ccordoba12 commented Nov 30, 2016 •

edited by goanpeca

Loading

ccordoba12 commented Dec 4, 2016 •

edited

Loading