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.

Sign up for GitHub

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

Jump to bottom

Add missing SMARTS strings and correctness tests for get_fx_groups() #73

Merged
merged 52 commits into from
Dec 14, 2023
Merged

Add missing SMARTS strings and correctness tests for get_fx_groups() #73

merged 52 commits into from
Dec 14, 2023

Conversation

Aariq
Copy link
Collaborator

@Aariq Aariq commented Oct 4, 2023 •
edited
Loading

Addresses #44 (partially) and #47

Adds SMARTS strings to capture all functional groups in the SIMPOL.1 paper except "number of carbons on the acid side of an amide" which I think is probably impossible with SMARTS alone. Adds a CSV file with test compounds and their expected functional group counts as well as a test to compare to that CSV.

This was linked to issues Oct 10, 2023
Missing SMARTS strings #44
Closed
Add correctness tests for get_fx_groups() #47
Closed
@Aariq Aariq removed a link to an issue Oct 10, 2023
Missing SMARTS strings #44
Closed
Aariq added 22 commits October 11, 2023 12:18
@Aariq
add nitroester pattern
87ba676
@Aariq
re-order to match paper
93c35e3
@Aariq
add pattern for carbonylperoxyacid
406ba24
@Aariq
add aromatic amine pattern
61a8212
@Aariq
add TNT as good negative example for various nitrogen compounds
3608d43
@Aariq
relax pattern for aromatic amine
5212909
@Aariq
(failed) attempt at nitrophenol pattern
51046fd
@Aariq
Merged origin/master into smarts
512c536
@Aariq
merge main
0728ecb 
Merge commit 'b91fce0a28ad7faeec5bd0417f1d1057055ddd3f'

#Conflicts:
#	tests/testthat/test-calc_vol.R
@Aariq
smarts for non-aromatic carbon double bonds
4f4bb1b
@Aariq
update news
c299585
@Aariq
capture C=C-C=O in a non-aromatic ring
197d4e4
@Aariq
add ether variations
7c17ca5
@Aariq
rename ether to ether_alkyl, use coalescing operator %||% to simplify…
c1e088b 
… code
@Aariq
ignore names
ace1571
@Aariq
strip names from columns created by chemminer
f55359c
@Aariq
fix aromatic ether pattern
cf5669f
@Aariq
fix hydroperoxide pattern to correct for peroxyacids
09db992
@Aariq
update NEWS
acbbddd
@Aariq
fix expected for L-DOPA
a218bf3
@Aariq
skip nitrophenols for now
938fd70
@Aariq
use .data pronoun
ee9fc72
@Aariq
Copy link
Collaborator Author

Aariq commented Nov 28, 2023

I think I'll want to "Squash and Merge" this one

Aariq added 4 commits November 30, 2023 10:47
@Aariq
use test_path() for easier interactive testing
fc8992c
@Aariq
merge main
5675496 
Merge commit '50371e2153752fefd0794c53dc3ea9feb8886c07'

#Conflicts:
#	NEWS.md
#	R/get_fx_groups.R
#	R/simpol1.R
#	man/get_fx_groups.Rd
@Aariq
check note
1e4e56c
@Aariq
Merged origin/master into smarts
c11df46
@Aariq

This comment was marked as outdated.

Sign in to view
Aariq
Aariq commented Dec 5, 2023
View reviewed changes
R/get_fx_groups.R Outdated Show resolved Hide resolved
@Aariq
Copy link
Collaborator Author

Aariq commented Dec 7, 2023 •
edited
Loading

Still need to:

  • double-check that all the functional groups have tests (i.e. the colnames in test_compounds.csv match)
  • make notes on test_compounds.csv about why each compound was chosen
  • add example compounds for secondary and tertiary amines and aldehydes
  • add example compounds for:
    • phosphoric_acid
    • phosphoric_ester
    • sulfate
    • sulfonate
    • thiol
    • carbothioester

@Aariq Aariq requested a review from KristinaRiemer December 8, 2023 23:11
@Aariq Aariq mentioned this pull request Dec 8, 2023
Closed
@Aariq Aariq linked an issue Dec 8, 2023 that may be closed by this pull request
sulfonate groups not getting captured? #93
Closed
@Aariq Aariq marked this pull request as draft December 8, 2023 23:17
@Aariq Aariq marked this pull request as ready for review December 8, 2023 23:34
@KristinaRiemer KristinaRiemer mentioned this pull request Dec 13, 2023
Closed
16 tasks
@KristinaRiemer KristinaRiemer merged commit 46005bb into master Dec 14, 2023
7 checks passed
@KristinaRiemer KristinaRiemer deleted the smarts branch December 14, 2023 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KristinaRiemer KristinaRiemer KristinaRiemer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

sulfonate groups not getting captured? Add correctness tests for get_fx_groups() Missing SMARTS strings
2 participants
@Aariq @KristinaRiemer