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

[FIX] Network Explorer: Clear selection on new network and when removing network #184

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Jan 29, 2021

Issue

Fixes #111.

Includes
  • Code changes
  • Tests

@codecov-io
Copy link

Codecov Report

Merging #184 (b4517a9) into master (247b3d8) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
+ Coverage   75.23%   75.26%   +0.03%     
==========================================
  Files          21       21              
  Lines        2790     2794       +4     
==========================================
+ Hits         2099     2103       +4     
  Misses        691      691              
Impacted Files Coverage Δ
orangecontrib/network/widgets/OWNxExplorer.py 77.97% <100.00%> (+0.19%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 247b3d8...b4517a9. Read the comment docs.

@VesnaT
Copy link
Contributor

VesnaT commented Feb 9, 2021

The Network Explorer widget does not work for me.
There seems to be a problem with _fr_layout.

@janezd
Copy link
Contributor Author

janezd commented Feb 11, 2021

I just tried with a fresh environment. In it

  • I installed orange3 from conda-forge
  • install cython
  • ran python setup.py develop in orange, base widget and canvas
  • ran pip install -e . in network add (I don't know why I used pip here... out of habit?)
    ... and everything works.

I don't know how to replicate and debug this. And I don't know what could be wrong, actually.

BTW, how does it doesn't work?

Copy link
Contributor

@VesnaT VesnaT left a comment

Choose a reason for hiding this comment

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

I guess, the cleared selection should result on the selected_data output as well, but it does not:
Connect Network File -> Network Explorer -> Data Table (with Selected Data) and select some points. After removing the Network File, selected data retains in Data Table. The same selected data is there after re-connecting Network File to Network Explorer with some other network.

This seems strange, since sending the selected data should be handled in parent widget.

self.assertIsNotNone(self.widget.selection)
self.send_signal(self.widget.Inputs.network, None)
self.assertEqual(self.widget.nSelected, 0)
self.assertIsNone(self.widget.selection)
Copy link
Contributor

@VesnaT VesnaT Feb 12, 2021

Choose a reason for hiding this comment

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

I prefer testing a widgets behavior to its properties. This test does not assure no selection on the output.
Adding
self.assertIsNone(self.get_output(self.widget.Outputs.selected_data))
results in a failed test.

@janezd janezd assigned janezd and unassigned VesnaT Feb 18, 2021
@janezd
Copy link
Contributor Author

janezd commented Feb 25, 2021

This seems strange, since sending the selected data should be handled in parent widget.

handleNewSignals does not call the inherited method because the derived widget is too different. (We have had similar problems in other widgets, too.)

The overriden version did not call unconditional_commit.

@janezd janezd removed their assignment Feb 25, 2021
@VesnaT
Copy link
Contributor

VesnaT commented Feb 26, 2021

When opening a saved workflow with selected instances, the selection is cleared. Is that intentional?

@janezd
Copy link
Contributor Author

janezd commented Feb 27, 2021

Here's another one: select some variable for Color. Then remove data. Variable remains selected.

I discovered this when fixing the pending selection. This widget does not properly reset itself because it does not call the inherited methods for this. The reason is, as I recall, that its signals are a bit more complicated. @VesnaT, when we can work together again, we should study this problem together.

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.

Network explorer keeps selection after data is removed
3 participants