sklvq icon indicating copy to clipboard operation
sklvq copied to clipboard

Added compatibility with correction matrix

Open NehaBari opened this issue 3 years ago • 3 comments

  1. Pulled changes from upstream to forked repo.
  2. Changed line 234 in _gmlvq.py to accept relevance correction as a parameter.

NehaBari avatar May 20 '21 14:05 NehaBari

Codecov Report

Merging #61 (3904477) into fr-51-null-space-correction (78e595c) will decrease coverage by 0.42%. The diff coverage is 22.22%.

Impacted file tree graph

@@                       Coverage Diff                       @@
##           fr-51-null-space-correction      #61      +/-   ##
===============================================================
- Coverage                        99.44%   99.01%   -0.43%     
===============================================================
  Files                               54       54              
  Lines                             1613     1622       +9     
  Branches                           139      142       +3     
===============================================================
+ Hits                              1604     1606       +2     
- Misses                               1        5       +4     
- Partials                             8       11       +3     
Impacted Files Coverage Δ
sklvq/models/_gmlvq.py 93.28% <22.22%> (-5.12%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c24c8ba...3904477. Read the comment docs.

codecov[bot] avatar May 20 '21 14:05 codecov[bot]

@NehaBari Good work! I changed it to a "draft" pull request, because some things need to happen before it can be accepted:

The codecov checks fail, this means that the test coverage of the project goes down and the method should be tested. The tests are located in the module's test folder (i.e., sklvq/models/tests). In the test_gmlvq.py file you should make a function called test_relevance_correction and add a simple example for which you know the answer (unit eigenvectors for example). See other test for inspiration.

Secondly, could you remove the old code (instead of commenting it out). We don't need the old (wrong) solution in the comments when using version control :-)

Thirdly, we should think about including an example in the documentation, i.e., an example that will be shown in sklvq.readthedocs.io. The examples that you see there are located here.

rickvanveen avatar May 21 '21 13:05 rickvanveen

Dear Rick,

Thankyou for the detailed feedback. Sounds good. I will start incorporating them.

NehaBari avatar May 21 '21 14:05 NehaBari