search

CDK-R / cdkr

Integrating R and the CDK
https://cdk-r.github.io/cdkr/
42 stars 27 forks source link

added runit.mass.R #87

Closed schymane closed 5 years ago

schymane commented 5 years ago

I have added the basis for unit tests for all functions for a few major cases but commented out all the (many) tests that are currently generating NPEs

schymane commented 5 years ago

I've reverted the incorrect changes (didn't realise they came all the way through to here automatically) and added some unit tests for the molecular formulas. These functions are returning inconsistent masses for the same option depending on input (mol vs MF) and different MF options have NPEs to the functions off mol. I've only added two sets of formula vs the 5 for mol as this is already quite a few inconsistencies to resolve already...

schymane commented 5 years ago

As for previous unit tests, I've commented out the ones that are either returning NPE or incorrect/inconsistent values (and just fixed a wrong NPE annotation).

schymane commented 5 years ago

Interesting, I definitely directed all the runit.mass.R changes into your massfuncs branch my side...

On Tue, Nov 13, 2018 at 3:59 AM +0100, "Rajarshi Guha" notifications@github.com<mailto:notifications@github.com> wrote:

Unfortunately, this pull request is for master which doesn't have the get.mass function

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/CDK-R/cdkr/pull/87#issuecomment-438112858, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AD4a_TMZ0HEqlowFJ0CU0bUu4VcTphpIks5uujVygaJpZM4YZ5_r.

rajarshi commented 5 years ago

Sorry, my bad - it was for the massfunc branch, and I've merged it in now. All is good