open-reaction-database / ord-schema

Schema for the Open Reaction Database

Apache License 2.0

95 stars 27 forks source link

Do not resolve NAME identifiers during submission #566

Closed skearnes closed 3 years ago

skearnes commented 3 years ago

We're hitting the resolution servers millions of times in https://github.com/open-reaction-database/ord-data/pull/59 (if I hadn't stopped the job early).

codecov[bot] commented 3 years ago

Codecov Report

Merging #566 (75ced80) into main (4297a12) will decrease coverage by 0.32%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #566      +/-   ##
==========================================
- Coverage   77.12%   76.80%   -0.33%     
==========================================
  Files          19       19              
  Lines        1937     1936       -1     
  Branches      473      473              
==========================================
- Hits         1494     1487       -7     
- Misses        299      301       +2     
- Partials      144      148       +4

Impacted Files	Coverage Δ
ord_schema/updates.py	`93.02% <100.00%> (-4.71%)`	:arrow_down:
ord_schema/resolvers.py	`70.78% <0.00%> (-4.50%)`	:arrow_down:

connorcoley commented 3 years ago

Aside from the flood of requests for this giant PR, are you opposed to using the name resolver upon submission in general?

skearnes commented 3 years ago

Aside from the flood of requests for this giant PR, are you opposed to using the name resolver upon submission in general?

I guess I don't feel strongly about that. It might be better to change the validations so that we warn when there are only NAME identifiers (that might be there already?) rather than actually adding the resolved SMILES during submission? I guess I'm thinking that we shouldn't be doing work that we should be pushing onto the submitters...

skearnes commented 3 years ago

PS part of my thinking here, besides the USPTO submission, is around whether contributors can verify changes. We do this resolution in the second phase of the submission where the contributor is not usually involved, and I think I'd prefer that these resolutions be done by the contributors themselves in the first phase instead.

connorcoley commented 3 years ago

PS part of my thinking here, besides the USPTO submission, is around whether contributors can verify changes. We do this resolution in the second phase of the submission where the contributor is not usually involved, and I think I'd prefer that these resolutions be done by the contributors themselves in the first phase instead.

This is convincing for me