search

CDK-R / cdkr

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

improve parse.smiles when invalid SMILES parsed #77

Closed adelenelai closed 5 years ago

adelenelai commented 5 years ago

added arguments omit.nulls and which.nulls to remove NULLs, and report which invalid SMILES were producing them respectively. Added a Warning to indicate how many invalid SMILES were parsed, if any. Warning comes independent of omit.nulls and which.nulls. Documentation edited accordingly.

Fixing issue #76

rajarshi commented 5 years ago

Hi @adelenelai, thanks for the pull request. Overall it looks good, except for the handling of which.nulls. Since the inclusion of this option results in a change to the return type (list of jobjRef's now becomes a two element list, this will break code that calls this function.

I would suggest to drop the which.nulls option, as it is trivial to compute that from the return value if omit.nulls=FALSE

adelenelai commented 5 years ago

Hi @rajarshi , thanks for feedback.

Removed which.nulls to preserve return type as a 1-element list as suggested.

rajarshi commented 5 years ago

Thanks, merged in