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

[WIP][ENH] Neighbors: data info displayed in status bar #4141

Closed
wants to merge 4 commits into from

Conversation

aturanjanin
Copy link
Contributor

@aturanjanin aturanjanin commented Oct 24, 2019

Description of changes

Fixes #4114 - Info box removed and instead Input and Output data displayed in status bar.

Includes
  • Code changes
  • Tests
  • Documentation

@VesnaT VesnaT self-assigned this Oct 24, 2019

@Inputs.data
def set_data(self, data):
self.data = data
self._set_label_text("data")
self._set_input_summary()
Copy link
Contributor

Choose a reason for hiding this comment

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

You can place this into handleNewSignals() instead. handleNewSignals() is emitted on any input change.


@Inputs.reference
def set_ref(self, refs):
self.reference = refs
self._set_label_text("reference")
self._set_input_summary()
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.


def _set_output_summary(self):
if self.data is None:
self.Outputs.data.send(None)
Copy link
Contributor

Choose a reason for hiding this comment

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

Outputting the data is already handled in apply(). Besides, it should not be sent twice, if not necessary.


indices = self._compute_indices()
if np.any(indices):
neighbors = self._data_with_similarity(indices)
Copy link
Contributor

Choose a reason for hiding this comment

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

Calculating the neighbors just for sake of the info is wasteful, and unnecessary.
The "info" code could be placed in an apply function, but if you insist to have a separate function for that, just pass it the neighbors calculated in the apply().

@@ -541,7 +541,7 @@ def __init__(self, parent=None, **kwargs):
fixedrb = QRadioButton("Fixed", checked=True)
group.addButton(fixedrb, RandomFeatureSelectEditor.Fixed)
kspin = QSpinBox(
minimum=1, value=self.__k,
minimum=1, maximum=10000000, value=self.__k,
Copy link
Contributor

Choose a reason for hiding this comment

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

This change should be removed from this PR.

elif self.data is not None:
n_data = len(self.data)
elif self.reference is not None:
n_refs = len(self.reference)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can shorten this by:

n_data = len(self.data) if self.data else 0
n_refs = len(self.reference) if self.reference else 0

and move the following else statement into the second if

details = f"No data instances on input; " \
f"{n_refs} reference instances on input "

self.info.set_input_summary(inst, details)
Copy link
Contributor

Choose a reason for hiding this comment

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

A shorter version:

        if n_data or n_refs:
            details = \
                f"{n_data if n_data else 'No'} data instance(s) on input\n" \
                f"{n_refs if n_refs else 'No'} reference instance(s) on input "
            self.info.set_input_summary(f"{n_data} | {n_refs} ", details)
        else:
            self.info.set_input_summary(self.info.NoInput)

@VesnaT
Copy link
Contributor

VesnaT commented Oct 25, 2019

Along with code changed, a test and documentation update should come.
In this case, fix the falling test in TestOWNeighbors will suffice, but the documentation should be updated since the appearance of the widget has changed.

@aturanjanin aturanjanin changed the title [ENH] Neighbors: data info displayed in status bar [WIP][ENH] Neighbors: data info displayed in status bar Oct 28, 2019
@aturanjanin aturanjanin closed this Nov 1, 2019
@aturanjanin aturanjanin deleted the owneighbors branch February 28, 2020 14:02
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.

Neighbors: remove Info box and display data info in status bar
2 participants