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

[ENH] Transform: Add new widget #3346

Merged
merged 2 commits into from
Nov 12, 2018
Merged

[ENH] Transform: Add new widget #3346

merged 2 commits into from
Nov 12, 2018

Conversation

VesnaT
Copy link
Contributor

@VesnaT VesnaT commented Oct 30, 2018

Issue

Implements #3342

Description of changes
Includes
  • Code changes
  • Tests
  • Documentation

@VesnaT VesnaT changed the title OWTransform: Add new widget [WIP]OWTransform: Add new widget Oct 30, 2018
@VesnaT VesnaT changed the title [WIP]OWTransform: Add new widget OWTransform: Add new widget Oct 30, 2018
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.

After you decide about the suggestion about handling the exception, I'd merge it unless @BlazZupan has any further comments.

def apply(self):
self.transformed_data = None
if self.data is not None and self.preprocessor is not None:
self.transformed_data = self.preprocessor(self.data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Preprocessor could raise an exception here. I think the widget has to catch and report it. Maybe something like "An error occurred while transforming data", and the second line (visible only on tooltip) would contain the exception message.

@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #3346 into master will increase coverage by 0.04%.
The diff coverage is 98.78%.

@@            Coverage Diff             @@
##           master    #3346      +/-   ##
==========================================
+ Coverage   82.21%   82.26%   +0.04%     
==========================================
  Files         351      353       +2     
  Lines       62349    62513     +164     
==========================================
+ Hits        51262    51426     +164     
  Misses      11087    11087

@VesnaT VesnaT changed the title OWTransform: Add new widget [ENH] Transform: Add new widget Nov 9, 2018
@lanzagar lanzagar added this to the 3.18 milestone Nov 9, 2018
@BlazZupan
Copy link
Contributor

Works as intended. Therefore merge. The documentation and subsequently a blog, once the widget enters the distribution, are missing. Also, for testing, could we release both widgets (PCA, Transform) into Prototypes (change name to, say, Test PCA, and Test Transform)?

@BlazZupan BlazZupan merged commit 8690622 into biolab:master Nov 12, 2018
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.

4 participants