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

added runit.mass.R #87

Merged
merged 10 commits into from Nov 13, 2018
Merged

added runit.mass.R #87

merged 10 commits into from Nov 13, 2018

Conversation

schymane
Copy link
Contributor

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

updating to Rajarshi's rearrangements
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
This reverts commit 6eafaf1 and other changes
Added formula tests - note that there are different NPEs and also inconsistent masses returned! See comments.
@schymane
Copy link
Contributor Author

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

fixed comment in wrong location
@schymane
Copy link
Contributor Author

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
Copy link
Contributor Author

schymane commented Nov 13, 2018 via email

@rajarshi
Copy link
Collaborator

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants