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] owpredicttions: Remove special magic value 2 #6933

Merged
merged 2 commits into from
Nov 29, 2024

Conversation

ales-erjavec
Copy link
Contributor

Issue

Fixes gh-6911

@janezd what was the point of using 2 as invalid value. I cannot find any place where it would be used/checked,
except where it is replaced with NaN on output.

Description of changes

Remove special magic value 2. Just use NaN.

Includes
  • Code changes
  • Tests
  • Documentation

Copy link

codecov bot commented Nov 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.46%. Comparing base (871b406) to head (de89d5f).
Report is 52 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6933      +/-   ##
==========================================
+ Coverage   88.39%   88.46%   +0.06%     
==========================================
  Files         329      331       +2     
  Lines       72480    73097     +617     
==========================================
+ Hits        64069    64663     +594     
- Misses       8411     8434      +23     
---- 🚨 Try these New Features:

@janezd janezd self-assigned this Nov 22, 2024
@janezd
Copy link
Contributor

janezd commented Nov 22, 2024

@janezd what was the point of using 2 as invalid value.

I'm guessing: the error column contained 2 as the error in probability estimation for discrete problems. In discrete data, this was used only for sorting. I probably wanted to put those values last, hence 2 made sense.

This became a problem after 2375dc7, which started using errorColumn for output.

This fix is OK if we can assume that numpy treats nan's as larger than ordinary numbers and will continue to do so, although I can't find this in documentation.

An alternative would be to do err[err == 2] = numpy.nan only for classification.

But the assumption is probably going to hold and your fix makes the code simpler.

@janezd janezd merged commit 9ff3124 into biolab:master Nov 29, 2024
12 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants