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

update protobuf version #660

Closed skearnes closed 1 year ago

codecov[bot] commented 1 year ago

Codecov Report

Merging #660 (441d81c) into main (842fa39) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #660   +/-   ##
=======================================
  Coverage   74.58%   74.58%           
=======================================
  Files          23       23           
  Lines        2723     2723           
  Branches      559      559           
=======================================
  Hits         2031     2031           
  Misses        583      583           
  Partials      109      109           
skearnes commented 1 year ago

Looks like the reason we pin to <3.20 is for compatibility with TensorFlow: https://github.com/tensorflow/tensorflow/blob/b59935d3b349e752eed4729606e5d64a17e48015/tensorflow/tools/pip_package/setup.py#L105

skearnes commented 1 year ago

Looks like the reason we pin to <3.20 is for compatibility with TensorFlow: https://github.com/tensorflow/tensorflow/blob/b59935d3b349e752eed4729606e5d64a17e48015/tensorflow/tools/pip_package/setup.py#L105

FYI @connorcoley @ipendlet @FanwangM we use TF in some of the examples

connorcoley commented 1 year ago

Is this a reason (or an excuse) to separate out the examples into their own repo? Seems like a complex dependency to have when it's not essential for many use cases

skearnes commented 1 year ago

Is this a reason (or an excuse) to separate out the examples into their own repo? Seems like a complex dependency to have when it's not essential for many use cases

That wouldn't fix the issue; if until TF supports protobuf>=3.20 we won't be able to use TF and ord-schema in the same interpreter since both require the proto libraries. I think our choices are (1) pin to whatever version of protobuf TF supports so that people can use TF, or (2) upgrade protobuf and declare that ord-schema cannot be used with TF.

FanwangM commented 1 year ago

I think it's not worth it to lose the support of TF because of version updating. We can just keep the current version and wait for TF to support protobug>=3.20.