-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 Refresh button to editors from Variable Explorer #21312
Conversation
The current plan is to add a toolbar (as suggested in #21315) and then have this be a button in the toolbar. This matches the interface for the Variable Explorer. |
Hello @jitseniesen! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2023-11-25 14:04:23 UTC |
I extended this a fair bit. Now it is possible to refresh array editors, dataframe editors, collection editors (for list, set, dict and tuple), and object explorers (for any type that is not handled by another editor). It works for editors opened directly from the variable explorer and also for editors opened from collection and object editors. The PR conflicts with PR #20546 and PR #21317, which add a toolbar to the dataframe editor and array editor respectively. The plan is to merge the two PRs that add a toolbar first, and then change this PR so that the Refresh button is added to the toolbar. |
Great work @jitseniesen! That'd be really nice to have. Ok, I'll try to review soon those PRs so you can finish this one. |
I rebased the PR in order to pick up the addition of the toolbar to the array and dataframe editors (thanks Carlos for the speedy review) and I updated the screenshots in the top comment. This is now ready for review. |
I rebased to pick up the CI automatic test fix in PR #21517. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jitseniesen ! I checked locally this and seems like things are working as expected 👍
Left a single comment regarding the safe_disconnect
function definition. Seems like taht function could be worthy to be moved to some utils
module for reusage (not only for this PR but seems like there are other places inside the source code from a quick search which could benefit from using it)
Other than that this LGTM 👍
spyder/plugins/variableexplorer/widgets/objectexplorer/objectexplorer.py
Outdated
Show resolved
Hide resolved
Can you provide the Videos in mp3 format, MIME format is unstable and unreadable on Windows 11 or Typically Every windows + xp Appreciated |
Regarding the tests not passing, maybe a rebase on top of the latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jitseniesen !
@dalthviz Thanks a lot for this suggestion. I had somehow convinced myself that rebasing could not possibly make a difference, so I kept looking for other explanations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jitseniesen for your work on this! I mostly left minor stylistic suggestions and improvements to docstrings for you.
spyder/plugins/variableexplorer/widgets/objectexplorer/objectexplorer.py
Show resolved
Hide resolved
Note: This also needs a rebase to get the fixes I did in PR #21553. |
This option was added in Spyder 2, but it is not used at the moment and may never been used. It interferes with the refresh functionality which is to be added in a following commit.
Make two functions. The first is setup_ui(), which sets up the UI and should be called only once. The second is set_data_and_check(), which specifies the data for the editor and can be called repeatedly. This is to prepare for the refresh functionality in the next commit.
* Add a function `CollectionsDelegate.make_data_function()` for retrieving the current value when the user refreshes an editor. This function returns `None`, indicating that the editor can not be refreshed. * Override `.make_data_function()` in `RemoteCollectionsDelegate`, so that if the function is called in the namespace editor in the variable explorer, it retrieves the current value of the variable currently selected in the variable explorer. * When creating a new array editor in `CollectionsDelegate`, pass the `data_function` that is created by `make_data_function` to the constructor. * Add a Refresh button to the toolbar of the array editor. When pressed, this calls the new function `.refresh()`. * This function calls `data_function` that was passed to the constructor and displays the return value in the array editor.
* In CollectionDelegate, pass `data_function` when creating a `CollectionEditor`. Pass this along to the `CollectionEditorWidget`. * Add a refresh button to the toolbar of `CollectionEditorWidget`, which raises a signal when pressed. Enable this button if `data_function` is set. * Add a function `CollectionEditor.refresh_editor()` and connect it to the new signal. In this function, check whether the user made any edits and if so, ask for confirmation first. Call `data_function` and use its return value as the new value to be displayed. Handle exceptions thrown by `data_function`. * Add tests for the new refresh functionality.
* Pass the `data_function` from `CollectionsEditor`, which can retrieve new data for the object being edited, via `CollectionsEditorTableView` to `CollectionsDelegate`. * Add `CollectionsDelegate.make_data_function()` for retrieving new data for editors opened from the collection editor. * Catch `IndexError` when calling a data_function, because this can be raised if an item in the collection is deleted. * Add tests for new functionality.
Make two functions. The first is setup_ui(), which sets up the UI and should be called only once. The second is set_data_and_check(), which specifies the data for the editor and can be called repeatedly. This is a refactor to prepare for the implementation of refresh functionality in the next commit.
* In CollectionDelegate, pass `data_function` when creating a dataframe editor. Pass this along to the `DataFrameView` * Add a refresh action to the `DataFrameView`, which raises a signal when it is triggered. Enable this action if `data_function` is set. * Add a function `DataFrameEditor.refresh_editor()` and connect it to the new signal. In this function, check whether the user edited the dataframe and if so, ask for confirmation first. Call `data_function` and use its return value as the new value for the dataframe to be displayed. Handle exceptions thrown by `data_function`, or when the new value is not a data frame. * Add tests for the new refresh functionality.
Make new function `ObjectExplorer.set_value()` for setting the object displayed in the object explorer, and call this from the constructor. This is a refactor to prepare for implementing the refresh functionality in the next commit.
* In CollectionDelegate, pass `data_function` when creating an object explorer. * Add a refresh toolbutton to the object explorer, which calls the new function `.refresh_editor()`. * In the new function, call `data_function` and use its return value as the new object to be displayed in the object explorer. Handle exceptions thrown by `data_function`. * Add tests for the new refresh functionality.
* Pass the `data_function` from the object explorer, which can retrieve new data for the object being edited, via `ToggleColumnTreeView` to `ToggleColumnDelegate`. * Add `ToggleColumnDelegate.make_data_function()` for retrieving new data for editors opened from the object explorer. * Pass the result of the above function to editors opened from the object explorer. * Add tests for new functionality.
For some reason, this test started to fail on Windows, probably because of the new menu item `Refresh`. This should fix the test.
* Fix style of refresh button in object explorer * Mock correct function in test * Many style fixes Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now. Thanks @jitseniesen for this terrific improvement!
Thank you so much to @jitseniesen and everyone else who worked hard to make this feature a reality!! |
Description of Changes
This adds a Refresh button to the array editor, the dataframe editor, the collection editor and the object explorer.
Clicking the Refresh button in the array editor refreshes the editor with the current value of the array variable:
refresh-std.mp4
The same for the dataframe editor:
refresh-dataframe.mp4
The same for the collection editor:
refresh-collection.mp4
The same of the object explorer:
refresh-object.mp4
It is possible to refresh editors opened from the Variable Explorer or editors opened from other editors, so the user can for instance refresh an array nested inside a dictionary inside an object inside another object inside a list:
refresh-nested.mp4
However, you can only refresh editors which ultimately originate from the Variable Explorer. For instance, you can open a collection editor from the IPython Console to display the contents of
sys.path
, but the Refresh button on that editor is disabled:refresh-disabled.mp4
If the user changes a value in the editor and then refreshes, Spyder asks for confirmation:
refresh-dirty.mp4
If the new value can't be displayed in the array editor, for instance because the variable now contains an integer, then Spyder shows an error message and closes the array editor:
refresh-invalid.mp4
The same happens if the variable no longer exists:
refresh-deleted.mp4
Note that in this case, the console displays an exception generated inside spyder-kernels. This is existing behaviour, the same happens in the current master if you open a variable in the Variable Explorer that no longer exists.
In the array editor, the UI changes depending on whether the array is a standard 2d array, a record array, a masked array or a 3d array. Refreshing the editor changes the UI if necessary:
refresh-to-3d.mp4
However, if the variable changes from an array to a dataframe, for instance, then the array editor will not transform into a dataframe editor. Instead, an informative message is displayed and the editor is closed, just as above where the array variable changed to an integer.
As part of this PR, I split the function which sets up the editors in two: one function for setting up the UI and one function for setting the value to be displayed. The first function is called only one, the second function is called when the editor is opened and when it is refreshed. I implemented the refresh functionality by passing another parameter,
data_function
, to the editors. Calling this function should return the current value of the variable; ifdata_function
isNone
then the editor can't be refreshed.The four editors (array, dataframe, collection and object) are all implemented differently, so the details of how to implement the refresh is also different in each case.
This PR removes the options
xlabels
andylabels
in the Array Editor. These can be used to change the column and row labels in the editor, but they are not currently used in Spyder. These options were added by Pierre in 2011. I could not find where they were used in the past, if they ever were, so I don't know their intended use. I don't know what to do when refreshing means that the size of the array changes, because then the number of labels also needs to change, so I chose to remove the options.Issue(s) Resolved
Fixes #6325
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct: Jitse Niesen