CDK-R / cdkr

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

Fix documentation and make test compatible with CDK v2.2 #72

Closed bachi55 closed 5 years ago

bachi55 commented 6 years ago

Hei,

when I ran R CMD check rcdk_3.4.9.tar.gz some tests failed:

....
Running the tests in ‘tests/doRUnit.R’ failed.
Last 13 lines of output:
  rcdk rcdk Unit Tests - 28 test functions, 0 errors, 3 failures
  FAILURE in test.frag3: Error in checkTrue(all(fworks == "org.openscience.cdk.AtomContainer")) : 
    Test not TRUE

  FAILURE in test.mcs1: Error in checkEquals("org.openscience.cdk.AtomContainer", .jclass(mcs)) : 
    1 string mismatch

  FAILURE in test.mcs3: Error in checkEquals("org.openscience.cdk.AtomContainer", .jclass(mcs)) : 
    1 string mismatch
...

I fixed those: Since CDK version 2.2 the default atom-container is AtomContainer2 (link).

There was also some documentation mismatch (compare with #71) that is fixed.

A third issue I could not fix (example in rcdk/man/atomcontainer.Rd):

  m <- parse.smiles('c1ccccc1')[[1]]

  ## Need to configure the molecule
  do.aromaticity(m)
  do.typing(m)
  do.isotopes(m)

  get.exact.mass(m)

throws an exception:

 Java-Object{java.lang.NullPointerException}"
Error in get.exact.mass(m) :
  Couldn't get exact mass. Maybe you have not performed aromaticity, atom type or isotope configuration?

The error remains also, when I perform the typing before the aromaticity detection (which probably is the correct order, but got mixed up in the examples). Maybe, we can figure this still out?

Best regards,

Eric

PS: I would like to make the tests pass, as I have some code I wanna contribute to the cdkr package. I have tests as well and in that way I found out about the mentioned issues.

bachi55 commented 5 years ago

Ping

rajarshi commented 5 years ago

Apologies for the silence - the day job has been hectic the last few months. I'll try and get to this on the weekend

On Fri, Sep 14, 2018 at 7:41 AM Eric Bach notifications@github.com wrote:

Ping

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rajarshi/cdkr/pull/72#issuecomment-421332903, or mute the thread https://github.com/notifications/unsubscribe-auth/AACGOYAV0vojEf4DtfjSkl6Pead0QQTzks5ua5XggaJpZM4VBQXZ .

-- Rajarshi Guha | http://blog.rguha.net | @rguha https://twitter.com/rguha

rajarshi commented 5 years ago

Merged this pull request. re the get.exact.mass issue, it's been reported in issue #73

Still not sure why that happens