Skip to content

Define PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION only when protobuf>=4.0.0 is used #4397

Merged
wyli merged 5 commits intoProject-MONAI:devfrom
harupy:conditionally-define-PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION
May 30, 2022
Merged

Define PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION only when protobuf>=4.0.0 is used #4397
wyli merged 5 commits intoProject-MONAI:devfrom
harupy:conditionally-define-PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION

Conversation

@harupy
Copy link
Copy Markdown
Contributor

@harupy harupy commented May 30, 2022

Fixes #4375

Description

A few sentences describing the changes proposed in this pull request.

As suggested in mlflow/mlflow#5968 (comment), PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python is only required in protobuf >= 4.0.0.

Status

Ready/Work in progress/Hold

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@harupy
Copy link
Copy Markdown
Contributor Author

harupy commented May 30, 2022

cc @wyli

@wyli
Copy link
Copy Markdown
Contributor

wyli commented May 30, 2022

Thanks @harupy, I understand the idea. Initially, I thought monai could remain agnostic about the probotuf versions (because monai doesn't directly depend on protobuf). Apparently now we couldn't avoid checking the version number of probobuf in the testing environments. Perhaps this is the only good way going forward.

harupy added 3 commits May 31, 2022 00:51
Signed-off-by: harupy <[email protected]>
Signed-off-by: harupy <[email protected]>
Signed-off-by: harupy <[email protected]>
@harupy harupy force-pushed the conditionally-define-PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION branch from ab546d9 to cbbf269 Compare May 30, 2022 15:51
@harupy harupy changed the title [DO NOT MERGE] Define PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION only when protobuf>=4.0.0 is used Define PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION only when protobuf>=4.0.0 is used May 30, 2022
Signed-off-by: harupy <[email protected]>
@wyli
Copy link
Copy Markdown
Contributor

wyli commented May 30, 2022

thank you very much @harupy, I'll merge this one.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented May 30, 2022

/build

@wyli wyli enabled auto-merge (squash) May 30, 2022 20:04
@wyli wyli merged commit 4dbbd86 into Project-MONAI:dev May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ERROR: test_metrics_track

2 participants