/cdkr

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

added runit.mass.R #87

Merged
merged 10 commits into from Nov 13, 2018
+112 −0

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@schymane @rajarshi
@schymane
Contributor

schymane commented Nov 12, 2018

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 added some commits Nov 10, 2018

@schymane
Merge pull request #2 from rajarshi/master 
updating to Rajarshi's rearrangements
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
2c42c84
@schymane
added get.molecular.weight
6b01c3b
@schymane
Merge pull request #3 from CDK-R/master 
latest updates
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
5d074e5
@schymane
Merge branch 'massfuncs' of https://github.com/CDK-R/cdkr into CDK-R-… 
…massfuncs
07f950d
@schymane
added runit.mass.R 
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
96697fc
@schymane
Update rcdk/R/mass.R
6eafaf1
@rajarshi

This comment has been minimized.

Sign in to view
Collaborator

rajarshi commented on 6eafaf1 Nov 12, 2018

Is this correct? I thought the JNI signature for object return values started with L. See Table 3.2

This comment has been minimized.

Sign in to view
Contributor

schymane replied Nov 12, 2018

nope, wasn't and is reverted.

schymane added some commits Nov 12, 2018

@schymane
Update rcdk/R/mass.R
17be8d9
@schymane
Revert "Update rcdk/R/mass.R" 
This reverts commit 6eafaf1 and other changes
41586e3
@schymane
Update rcdk/inst/unitTests/runit.mass.R 
Added formula tests - note that there are different NPEs and also inconsistent masses returned! See comments.
24c0b2d
@schymane

This comment has been minimized.

Sign in to view
Contributor

schymane commented Nov 12, 2018

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
Update rcdk/inst/unitTests/runit.mass.R 
fixed comment in wrong location
4af2861
@schymane

This comment has been minimized.

Sign in to view
Contributor

schymane commented Nov 12, 2018

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

@rajarshi rajarshi merged commit 5f60220 into CDK-R:massfuncs Nov 13, 2018

@schymane

This comment has been minimized.

Sign in to view
Contributor

schymane commented Nov 13, 2018

@rajarshi

This comment has been minimized.

Sign in to view
Collaborator

rajarshi commented Nov 13, 2018

Sorry, my bad - it was for the massfunc branch, and I've merged it in now. All is good
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment