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] OWEditDomain: Clear editor when data is disconnected #4484

Merged
merged 1 commit into from
Mar 20, 2020

Conversation

aturanjanin
Copy link
Contributor

Issue

Fixes #4483

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #4484 into master will increase coverage by 0.22%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4484      +/-   ##
==========================================
+ Coverage   83.16%   83.39%   +0.22%     
==========================================
  Files         268      274       +6     
  Lines       54001    55077    +1076     
==========================================
+ Hits        44911    45930    +1019     
- Misses       9090     9147      +57     

Copy link
Contributor

@janezd janezd left a comment

Choose a reason for hiding this comment

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

Nice. It's a small glitch, but we should tolerate them.

I don't entirely like the fix, though. It relies on the left-hand listview sending an (essentially invalid) signal that something (in fact: nothing) has been selected. The solution is thus based on a side-effect.

I think this should be explicitly handled by OWEditDomain.clear whose job is to clean up the widget. I suppose OWEditDomain.clear should call self._editor.clear(), where ReinterpretVariableEditor.clear would probably be defined as:

    def clear(self):
        self.layout().currentWidget().clear()

This seems to work, but please check - in particular for any crashes for different current widgets. (Can current widget be None or something else that doesn't have clear?)

Also think whether ReinterpretVariableEditor.clear also needs to do something else. Also check that the corresponding widget's clear methods do everything they need to to clean their houses.

@janezd
Copy link
Contributor

janezd commented Mar 5, 2020

Line 1: "should": I meant "shouldn't"! :)

@janezd janezd self-assigned this Mar 5, 2020
@aturanjanin aturanjanin changed the title [FIX] OWEditDomain: Clear editor when data is disconnected [WIP] OWEditDomain: Clear editor when data is disconnected Mar 6, 2020
@aturanjanin aturanjanin closed this Mar 9, 2020
@aturanjanin aturanjanin deleted the oweditdomain branch March 9, 2020 09:36
@aturanjanin aturanjanin restored the oweditdomain branch March 11, 2020 08:24
@aturanjanin aturanjanin reopened this Mar 11, 2020
@aturanjanin
Copy link
Contributor Author

What if we add self.layout().currentWidget().clear() call into the already existing function OWEditDomain.clear_editor and call that function in the OWEditDomain.clear, would that be a fine solution maybe?
Also, current widget can only be one of four - Discrete/Continuous/Time/Variable Editor.

@janezd
Copy link
Contributor

janezd commented Mar 13, 2020

OK, you can do so, too, I guess.

@aturanjanin aturanjanin reopened this Mar 18, 2020
@aturanjanin aturanjanin changed the title [WIP] OWEditDomain: Clear editor when data is disconnected [FIX] OWEditDomain: Clear editor when data is disconnected Mar 18, 2020
@janezd janezd merged commit 0eedf16 into biolab:master Mar 20, 2020
@aturanjanin aturanjanin deleted the oweditdomain branch March 30, 2020 14:46
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.

EditDoman doesn't clear editor when data is disconnected
2 participants