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] preprocess: Fix RemoveNaNClasses / Use existing HasClass #2450

Merged
merged 3 commits into from
Jul 10, 2017

Conversation

lanzagar
Copy link
Contributor

@lanzagar lanzagar commented Jul 5, 2017

Issue

SklLearner methods do not work on SqlTable data because of the RemoveNaNClasses preprocessor.

Description of changes

Use an existing (more general) implementation of the RemoveNaNClasses preprocessor - the HasClass filter.
Why do we even have both (functionality looks exactly the same)?

Before merging, we should check whether the RemoveNaNClasses preprocessor can be removed completely and HasClass filter used in its place.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov-io
Copy link

codecov-io commented Jul 5, 2017

Codecov Report

Merging #2450 into master will increase coverage by 0.4%.
The diff coverage is 88%.

@@           Coverage Diff            @@
##           master   #2450     +/-   ##
========================================
+ Coverage    73.9%   74.3%   +0.4%     
========================================
  Files         321     321             
  Lines       55993   56021     +28     
========================================
+ Hits        41379   41627    +248     
+ Misses      14614   14394    -220

@lanzagar lanzagar force-pushed the removenancls branch 3 times, most recently from 4c294fa to 4444e49 Compare July 7, 2017 12:10
@lanzagar lanzagar changed the title [WIP] preprocess: Fix RemoveNaNClasses / Use existing HasClass [FIX] preprocess: Fix RemoveNaNClasses / Use existing HasClass Jul 7, 2017
@@ -197,6 +198,7 @@ def __call__(self, data):
return data.transform(domain)


@deprecated("Orange.data.filter.HasClas")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

HasClas -> HasClass

@astaric astaric merged commit feea5ef into biolab:master Jul 10, 2017
@lanzagar lanzagar deleted the removenancls branch March 14, 2022 14:41
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.

3 participants