open-reaction-database / ord-schema

Schema for the Open Reaction Database
https://open-reaction-database.org
Apache License 2.0
92 stars 26 forks source link

Fix problem of generating dataset when having `reaction_id` in the template (#697) #699

Closed FanwangM closed 10 months ago

FanwangM commented 11 months ago

The PR is an attempt to fix #697 by filtering out the line with reaction_id.

codecov[bot] commented 11 months ago

Codecov Report

Merging #699 (17effeb) into main (4961279) will increase coverage by 0.02%. Report is 1 commits behind head on main. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #699      +/-   ##
==========================================
+ Coverage   69.33%   69.36%   +0.02%     
==========================================
  Files          23       23              
  Lines        2322     2324       +2     
  Branches      589      590       +1     
==========================================
+ Hits         1610     1612       +2     
  Misses        598      598              
  Partials      114      114              
Files Changed Coverage Δ
ord_schema/templating.py 95.31% <100.00%> (+0.15%) :arrow_up:
FanwangM commented 10 months ago

Instead of doing string operations on the template pbtxt, it would be cleaner to remove the field from the dataset after it is generated by the enumeration, I think. WDYT?

This can be a cleaner solution. New changes are committed here.

One question, do we need to assign a reaction_id for each reaction in the database after the reaction list is constructed?

connorcoley commented 10 months ago

One question, do we need to assign a reaction_id for each reaction in the database after the reaction list is constructed?

No, the submission workflows will automatically assign a reaction_id to each