Overly strict Python dependency requirements about fhir HOT 11 CLOSED

Hey! 👋

A few more changes need to go in before this is tested and cut (0.7.3 has not been cut yet, though some of the changes around further loosening Python dependencies are pushed to GitHub).

If this is time-sensitive, you can clone+build the package artifacts from source to test out the pypi package.

from fhir.

Most of this stems from the bazel-compatibility story. In particular, we want to be sure that the packages installed and leveraged when bazel (build/test/run)ing are identical to what you get via pip install. However, bazel+pip interoperability (as of rules_python 0.0.2) presents some difficulties here w.r.t. transitive dependency resolution (e.g. see #125, #102).

Because of this, at the moment we are opting for explicit versions to promote determinism and safety. I've been experimenting with the new rules_python release (0.1.0) which should resolve the transitive dependency issues along with a few other bugs related to bazel+pip, but it introduces some changes into how Python3 implicit namespace packages are being handled (converting implicit namespace packages to pkg-util namespace packages). Once I can ensure this is working safely+properly we'll try relaxing these constraints to something more permissible!

from fhir.

Ah, I gotcha. That makes sense. Thanks for explaining. Bazel seems tricky to work with.

from fhir.

Hey! Have you had any luck on this by chance? We're trying to use mypy-protobuf 2.4 which should give grpc stub generation for stricter type checking.

If not, would it be possible to bump the explicit dependencies to the current latest versions?

Thank you!

Edit: Does look like the version is getting installed anyway, so this may not hinder us after all.

from fhir.

Hey @gitpushdashf -- I spent some time looking at this a couple of days ago.

With regards to mypy-protobuf, we only actually leverage the plugin when building the distribution from source and packaging the resulting stubs. As such, I don't think this really belongs in requirements.txt (see here). I've made a change to move this out of requirements.txt and into build_distribution.sh. In the meantime, it shouldn't be a problem to install your desired version (and it seems like from your comment this worked out!).

On a related note, I spent time verifying rules_python 0.1.0 compatibility with our package namespace structure. It seems to be handling the package namespaces correctly for bazel, so I created a change to adopt this, and subsequently move requirements.txt to:

absl-py~=0.10.0
backports.zoneinfo~=0.2.1
protobuf~=3.13.0
python-dateutil~=2.8.1

Which omits the transitive deps of these packages and allows for compatible versions to be installed.

Both of these changes are under review.

from fhir.

Ah, that's fantastic! Thank you. I didn't know about the ~= in requirements.txt.

from fhir.

Hey!

Unfortunately, looks like this isn't working quite as expected. The patch level looks to be adjustable, but that's all.

I seem to get this with Pipenv or Poetry.

package $ poetry update
Updating dependencies
Resolving dependencies... (11.7s)

  SolverProblemError

  Because google-fhir (0.7.2) depends on protobuf (>=3.13.0,<3.14.0)
   and no versions of google-fhir match >0.7.2,<0.8.0, google-fhir (>=0.7.2,<0.8.0) requires protobuf (>=3.13.0,<3.14.0).
  So, because package depends on both protobuf (^3.17.3) and google-fhir (^0.7.2), version solving failed.

  at /usr/local/lib/python3.8/site-packages/poetry/puzzle/solver.py:241 in _solve
      237│             packages = result.packages
      238│         except OverrideNeeded as e:
      239│             return self.solve_in_compatibility_mode(e.overrides, use_latest=use_latest)
      240│         except SolveFailure as e:
    → 241│             raise SolverProblemError(e)
      242│
      243│         results = dict(
      244│             depth_first_search(
      245│                 PackageNode(self._package, packages), aggregate_package_nodes

I wonder if protobuf =< 3.13.0 > 4 would work.

from fhir.

Hi @gitpushdashf 👋,

Apologies, I should have been more explicit that a13c306 was intended to only allow fluctuations in patch version.

After further review, for mature packages we should be fine to also free this up to minor/patch version, which should fix your dependency resolution problem.

E.g. requirements.txt now becomes:

absl-py~=0.10.0
backports.zoneinfo~=0.2.1;python_version<"3.9"
protobuf~=3.13
python-dateutil~=2.8

See PEP0440 for more details on ~=.

This is in review.

from fhir.

Ohh, that is interesting. I didn't realize that ~=X.Y.Z behaved so differently from ~=X.Y.

This sounds great. We're curious to try the newer protobuf version in particular, since 3.13.0 has been out for a while.

Thank you!

from fhir.

Hey, I see that you guys cut 0.7.3 but haven't seen it on pypi yet. Is it coming soon?

Thank you!

from fhir.

Ah, I understand. I just saw the 0.7.3 in the Python sources, I guess.

Thank you!

from fhir.