search

rickvanveen / sklvq

http://sklvq.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
49 stars 9 forks source link

Added compatibility with correction matrix #61

Closed NehaBari closed 1 week ago

NehaBari commented 3 years ago
  1. Pulled changes from upstream to forked repo.
  2. Changed line 234 in _gmlvq.py to accept relevance correction as a parameter.
codecov[bot] commented 3 years ago

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 99.01%. Comparing base (9318944) to head (3904477). Report is 76 commits behind head on fr-51-null-space-correction.

Files with missing lines Patch % Lines
sklvq/models/_gmlvq.py 0.00% 2 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## fr-51-null-space-correction #61 +/- ## =============================================================== - Coverage 99.29% 99.01% -0.28% =============================================================== Files 52 54 +2 Lines 1550 1622 +72 Branches 133 142 +9 =============================================================== + Hits 1539 1606 +67 - Misses 1 5 +4 - Partials 10 11 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rickvanveen commented 3 years ago

@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.

NehaBari commented 3 years ago

Dear Rick,

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