Dependencies: Bump a few versions, start using version range constraints #1017

amotl · 2023-09-29T12:37:31Z

About

Related to GH-1015 and GH-1016, this patch intends to modernize a few dependencies.

Details

I've verified the installation works using this command. I did not run any software tests yet.

pip install --editable='.[export,influxdb,cratedb,postgresql,radar,bufr,restapi,explorer,radar,radarplus]' --prefer-binary

amotl · 2023-09-29T14:39:46Z

Problem

After mitigating a few woes, the patch is now down to four failures, related to imgw. I can reproduce them locally by using this command:

pytest -k test_imgw_meteorology_api_monthly

The error is:

exceptions.ComputeError: strict conversion from `str` to `f64` failed for value(s) ["1835 31"]; if you were trying to cast Utf8 to temporal dtypes, consider using `strptime`

-- https://github.com/earthobservations/wetterdienst/actions/runs/6352977539/job/17256790304?pr=1017#step:8:909

Q&A

@gutzbenj: Do you think it is related to my version bumps, or do you think it could be unrelated?

gutzbenj · 2023-09-29T14:43:10Z

Dear @amotl ,

looking at the changes I wonder what effect they have as most version constraints are the same as before but some libraries are even more constrained with the lower upper bound.

gutzbenj · 2023-09-29T14:44:21Z

The issues with the tests are related to actual issues in the data!

amotl · 2023-09-29T14:47:55Z

looking at the changes I wonder what effect they have as most version constraints are the same as before

It will give people installing Wetterdienst as a library much more freedom. Before, we mostly used ^xx.yy constraint definitions, which nailed the dependencies to the very release it was designated at, modulo semver headroom.

but some libraries are even more constrained with the lower upper bound.

It will give everyone more safety even when time progresses, i.e. each version of Wetterdienst will be more likely to work, even in the future, while Dependabot is the driver not to stay in the past for too long. With NumPy and pandas, I was just playing around until I realized I needed to bump duckdb. Both commits can probably be removed again.

gutzbenj · 2023-09-29T14:54:33Z

I'm not sure that's the case. When you look at the poetry docs [1] e.g. ^1.2.3 equals >=1,<2. I guess what you mean is ~1.2.3 which equals >=1.2.3,<1.3

[1] https://python-poetry.org/docs/dependency-specification/

amotl · 2023-09-29T14:56:32Z

I'm not sure that's the case.

What exactly?

When you look at the poetry docs [1] e.g. ^1.2.3 equals >=1,<2.

What would ^0.7.1 equal to?

gutzbenj · 2023-09-29T14:58:43Z

The constraints that are currently set mostly equal the changes you set.

That would equal >=0.7.1,<1.0

amotl · 2023-09-29T14:59:24Z

I guess what you mean is ~1.2.3 which equals >=1.2.3,<1.3.

Ah, do you suggest to switch everything over to semver instead? I would be open to, but still do not feel totally convinced and comfortable, and made good experiences with version ranges in the past, that's why I was advocating for them.

gutzbenj · 2023-09-29T15:00:07Z

Oh my bad!!! I misread the docs and what I said applies only after 1.0 as it looks. What we have to do is reduce the version constraint to second level e.g. ^0.1.2 we have to change to ^0.1.

amotl · 2023-09-29T15:01:33Z

^0.7.1 would equal >=0.7.1,<1.0.

This is why I love version ranges, because you can define that "it works" with higher major version numbers like >=0.7.1,<2. I can't do that with any other notation, other than enumerating different ranges of major versions explicitly, right?

amotl · 2023-09-29T15:04:37Z

Oh my bad!!! I misread the docs and what I said applies only after 1.0 as it looks. What we have to do is reduce the version constraint to second level e.g. ^0.1.2 we have to change to ^0.1.

With all those special cases which tend to be regularly forgotten, or not at hand at the right time, it is another reason why I prefer the explicit version range notation.

kmuehlbauer · 2023-09-29T15:06:11Z

So, does that mean, the Pint pinning had an issue being restricted to 0.17.X.

But for numpy anything in the 1.X.Y should be allowed?

Then, for numpy it might just be a wrong translation of the caret pinning over at conda-forge.

kmuehlbauer · 2023-09-29T15:11:04Z

The docs say, the caret pinning preserves the leftmost non-zero value.
So ^0.17 will clearly stay <0.18 or am I missing something?

amotl · 2023-09-29T15:21:25Z

So ^0.17 will clearly stay <0.18 or am I missing something?

Yes, I think this is correct, and needs to be relaxed.

But for numpy anything in the 1.X.Y should be allowed?

In theory, but the interdependencies of NumPy with other libraries like pandas and transiently to duckdb are more subtle, and sometimes problems only trip on behalf of a larger application and test cases like we are running here.

I think it is safer to explicitly let version combinations be validated by CI, and driven by Dependabot. Once a package is released, we do not have any longer the chance to fiddle with its defined dependencies, and with limiting on the upper bound, we are taking care that a package is sound when being installed, even in the future.

This specifically applies to "huge" dependencies with many changes even between their minor releases, like NumPy or pandas. The situation is definitively more relaxed with other packages having a more infrequent release cycle, with less changes.

gutzbenj · 2023-09-29T15:31:57Z

According to poetry docs ^0.17 means <1.0 but ^0.17.X means <0.18. I made the mistake when adding new dependencies that I didn't remove the third patch version layer from the string after adding with poetry add.

amotl · 2023-09-29T15:34:56Z

pyproject.toml

-webdriver-manager = "^4.0.0"
+webdriver-manager = ">=4,<5"


Let me use this as an example to demonstrate why I prefer the version range notation: When version 5 of this package will be released, Dependabot will submit a change to >=4,<6, and the outcome from CI will immediately tell whether this is good or not. If version 5 works, and just because it is version 5 already, I am trusting their maintainers enough that it will not break during the whole version 5 release cycle.

On the other hand, a "point" bump would raise the lower version bound of this package to ">=5", which is an unneccessary restriction if it still works with version 4, and on the other hand, too dangerous without an upper bound, because nobody knows what version 6 will be all about.

Those thoughts include a series of assumptions and decisions I would like to make individually per dependency. They hopefully outline my thoughts why I am so much in favor of version range notations. Of course, it only explains my thoughts on behalf of a single example, but I am sure you can extrapolate from that.

amotl · 2023-09-29T15:36:07Z

pyproject.toml

+measurement = ">=3.2,<4"
+numpy = ">=1.22,<1.24"
+pandas = ">=2,<2.1"
+Pint = ">=0.17,<0.23"


If it technically works with Pint, because the package is compatible enough, I believe it is the right way to write down dependencies in ranges of version numbers, instead of anything else - modulo exceptions ;].

Only writing >=0.17 would be dangerous, because it is very likely to break until a 1.x release.

amotl · 2023-09-29T15:38:53Z

According to poetry docs ^0.17 means <1.0 but ^0.17.X means <0.18. I made the mistake when adding new dependencies that I didn't remove the third patch version layer from the string after adding with poetry add.

Thanks for adding yet another reason why I don't like the convenience variants for version notations, even if they sound convenient. There are just too many troubles, in past, presence, and future.

amotl · 2023-09-29T15:43:31Z

I also wanted to add that the resolver now completes in just a few seconds 💯. I am sure there have been advancements in Poetry to increase performance, but I am equally sure it now has much more headroom with those relaxed constraints.

gutzbenj · 2023-09-29T15:56:42Z

Happy merging! 🙂

amotl · 2023-09-29T16:43:34Z

tests/provider/imgw/meteorology/test_api.py

+# FIXME: Deactivated after flaw in data discovered on 2023-09-26
+@pytest.mark.skip(reason="Deactivated after flaw in data discovered on 2023-09-26")


Will you separately take care about this regression, @gutzbenj?

amotl · 2023-09-29T16:43:43Z

tests/provider/imgw/meteorology/test_api.py

+# FIXME: Deactivated after flaw in data discovered on 2023-09-26
+@pytest.mark.skip(reason="Deactivated after flaw in data discovered on 2023-09-26")


amotl · 2023-09-29T16:43:51Z

tests/test_api.py

-        ("imgw", "hydrology", {"parameter": "hydrology", "resolution": "daily"}, None),
+        # FIXME: Deactivated after flaw in data discovered on 2023-09-26
+        pytest.param(
+            "imgw", "hydrology", {"parameter": "hydrology", "resolution": "daily"}, None, marks=pytest.mark.xfail
+        ),
        # IMGW Meteorology
-        ("imgw", "meteorology", {"parameter": "climate", "resolution": "daily"}, "249200180"),
+        pytest.param(
+            "imgw", "meteorology", {"parameter": "climate", "resolution": "daily"}, "249200180", marks=pytest.mark.xfail
+        ),


amotl · 2023-09-29T17:15:04Z

Hmm. Did you see that one before, @gutzbenj?

ERROR tests/ui/explorer/test_explorer.py::test_app_layout - AttributeError: module 'percy' has no attribute 'Runner'

I don't think dependency adjustments changed anything significant here. At least, when rolling back the changes outlined below, there are no changes to poetry.lock on my machine.

Before:

percy-selenium = "^2.0.0"
selenium = "^4.0"

Now:

percy-selenium = ">=2,<3"
selenium = ">=4,<5"

-- https://github.com/earthobservations/wetterdienst/actions/runs/6354438040/job/17260902996?pr=1017#step:8:902

amotl · 2023-09-29T17:41:13Z

pytest tests/ui/explorer/test_explorer.py -k test_app_layout

pip install "percy<2" magically makes the test case succeed on my machine.

However, the Poetry resolver reports this is not ok:

So, because wetterdienst depends on both dash (^2.6) and percy (<2), version solving failed.

amotl · 2023-09-29T18:25:29Z

Don't know what is going on about Percy. fb26a9f may fix it.

Old: https://github.com/percy/python-percy-client (pip install percy)
New: https://github.com/percy/percy-selenium-python (pip install percy-selenium)

However, installing vanilla percy after installing percy-selenium fixes the problem for me. Maybe they need to tune their packaging a bit?

amotl · 2023-09-29T18:33:35Z

.github/workflows/install.sh

+# FIXME: Remove this again.
+# Fix dependency woes about percy: AttributeError: module 'percy' has no attribute 'Runner'.
+# https://github.com/earthobservations/wetterdienst/pull/1017#issuecomment-1741240635
+# pytest tests/ui/explorer/test_explorer.py -k test_app_layout
+pip install "percy>=2" --force


Q: What's that?
A: It fixes a problem running the "explorer" tests on my machine, and hopefully also on CI. See my report at #1017 (comment) ff.

amotl · 2023-09-29T18:33:52Z

.github/workflows/coverage.yml

+          cache-dependency-path: |
+            pyproject.toml
+            poetry.lock


Better safe than sorry. ;]

amotl · 2023-09-29T19:27:16Z

Problem

Only on Windows:

Installing the current project: wetterdienst (0.60.0)
+ poetry run -- python -m pip install 'percy>=2,<3' --force
The system cannot find the file specified.
Error: Process completed with exit code 1.

-- https://github.com/earthobservations/wetterdienst/actions/runs/6356128387/job/17265271480?pr=1017#step:6:296

References

poetry run causes [FileNotFoundError] with some scripts python-poetry/poetry#2481
Poetry on Windows fails with File does not exists error python-poetry/poetry#4479

Fix?

poetry run poe fix-percy

[tool.poe.tasks]
fix-percy = "python -m pip install 'percy>=2,<3' --force"

amotl · 2023-09-29T19:57:54Z

This patch has quite a few quirks [1]. Still, a lot of debugging went into it, and it seems to be "right" now, despite including workarounds, so I would like to bring it in, in order to continue with more version bumping.

[1]: One of them, I've reported upstream to percy/percy-selenium-python#66.

amotl · 2023-09-29T20:03:27Z

Oh my. Tests on Windows fail at the very end with an extraordinarily classic error.

MemoryError: Unable to allocate output buffer.

-- https://github.com/earthobservations/wetterdienst/actions/runs/6356384858/job/17265893861?pr=1017#step:8:833

Maybe just a hiccup while acquiring MOSMIX data. Re-running with 🤞.

kmuehlbauer

@amotl Let's put trust into this debugging session and apply these pinnings over at conda-forge.

I can do this, but on Monday earliest.

Thanks for all the hard work here!

amotl · 2023-09-29T20:20:57Z

Did you ever observe such errors revolving around xdist, when running tests on Windows, @gutzbenj?

[gw1] node down: Not properly terminated
[gw1] [ 93%] FAILED tests/ui/test_cli.py::test_cli_values_json_tidy[dwd-observation---resolution=daily --parameter=kl --date=2020-06-30-station_id0-Dresden-Klotzsche] 

replacing crashed worker gw1

-- https://github.com/earthobservations/wetterdienst/actions/runs/6356384858/job/17266345457?pr=1017#step:8:718

codecov · 2023-09-29T20:32:23Z

Codecov Report

Attention: 138 lines in your changes are missing coverage. Please review.

Comparison is base (bab5797) 90.91% compared to head (8df10dc) 89.69%.
Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1017      +/-   ##
==========================================
- Coverage   90.91%   89.69%   -1.23%     
==========================================
  Files         105      103       -2     
  Lines        8718     9498     +780     
  Branches      998     1056      +58     
==========================================
+ Hits         7926     8519     +593     
- Misses        592      766     +174     
- Partials      200      213      +13

Flag	Coverage Δ
unittests	`89.69% <89.43%> (-1.23%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
wetterdienst/api.py	`92.92% <100.00%> (+0.32%)`	⬆️
wetterdienst/core/timeseries/export.py	`89.17% <ø> (ø)`
wetterdienst/core/timeseries/request.py	`86.27% <ø> (ø)`
wetterdienst/core/timeseries/tools.py	`76.00% <ø> (ø)`
wetterdienst/provider/dwd/dmo/__init__.py	`100.00% <100.00%> (ø)`
wetterdienst/provider/dwd/mosmix/__init__.py	`100.00% <ø> (ø)`
wetterdienst/provider/dwd/mosmix/api.py	`95.17% <100.00%> (+10.73%)`	⬆️
wetterdienst/provider/dwd/observation/download.py	`75.75% <ø> (ø)`
wetterdienst/provider/dwd/observation/fields.py	`82.75% <ø> (ø)`
wetterdienst/provider/dwd/radar/api.py	`95.14% <ø> (ø)`
... and 10 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

amotl force-pushed the collab/modernize-dependencies branch 2 times, most recently from 0511495 to 683b1ed Compare September 29, 2023 14:24

amotl commented Sep 29, 2023

View reviewed changes

amotl force-pushed the collab/modernize-dependencies branch 2 times, most recently from 2a95e36 to e293ba2 Compare September 29, 2023 16:30

amotl added 3 commits September 29, 2023 18:32

Tests: Skip IMGW meteorology tests because of a flaw in data

b26e3d2

Dependencies: Bump a few versions, start using version range constraints

750e3b5

Dependencies: Use duckdb>=0.9,<0.10

3118817

amotl force-pushed the collab/modernize-dependencies branch from e293ba2 to 3118817 Compare September 29, 2023 16:34

amotl commented Sep 29, 2023

View reviewed changes

amotl mentioned this pull request Sep 29, 2023

Too strict dependency pinning #1015

Closed

amotl added 2 commits September 29, 2023 19:50

Dependencies: Use dash>=2.10,<2.14

4cd6ffd

CI: Configure cache-dependency-path for actions/setup-python@v4

c7c0adf

amotl commented Sep 29, 2023

View reviewed changes

amotl force-pushed the collab/modernize-dependencies branch from fb26a9f to e43181e Compare September 29, 2023 18:43

This comment was marked as resolved.

Sign in to view

amotl force-pushed the collab/modernize-dependencies branch 3 times, most recently from 929fe1b to 37a2424 Compare September 29, 2023 19:04

amotl mentioned this pull request Sep 29, 2023

AttributeError: module 'percy' has no attribute 'Runner' percy/percy-selenium-python#66

Closed

amotl force-pushed the collab/modernize-dependencies branch from 37a2424 to b235d16 Compare September 29, 2023 19:18

amotl force-pushed the collab/modernize-dependencies branch 2 times, most recently from 39dc10e to f3cedcb Compare September 29, 2023 19:38

Dependencies: AttributeError: module 'percy' has no attribute 'Runner'

8df10dc

amotl force-pushed the collab/modernize-dependencies branch from f3cedcb to 8df10dc Compare September 29, 2023 19:44

amotl requested review from kmuehlbauer and gutzbenj September 29, 2023 19:59

kmuehlbauer reviewed Sep 29, 2023

View reviewed changes

amotl marked this pull request as ready for review September 29, 2023 20:37

amotl merged commit 8520850 into main Sep 29, 2023
17 checks passed

amotl deleted the collab/modernize-dependencies branch September 29, 2023 20:38

This was referenced Sep 29, 2023

Dependencies: Bump a few more versions using version range constraints #1018

Merged

CI: Collection of flukes #816

Open

Dependencies: Bump leftover versions using version range constraints #1020

Merged

		# FIXME: Deactivated after flaw in data discovered on 2023-09-26
		@pytest.mark.skip(reason="Deactivated after flaw in data discovered on 2023-09-26")

		webdriver-manager = "^4.0.0"
		webdriver-manager = ">=4,<5"

Dependencies: Bump a few versions, start using version range constraints #1017

Dependencies: Bump a few versions, start using version range constraints #1017

Conversation

amotl commented Sep 29, 2023 • edited

About

Details

amotl commented Sep 29, 2023

Problem

Q&A

gutzbenj commented Sep 29, 2023

gutzbenj commented Sep 29, 2023

amotl commented Sep 29, 2023 • edited

gutzbenj commented Sep 29, 2023 • edited

amotl commented Sep 29, 2023

gutzbenj commented Sep 29, 2023

amotl commented Sep 29, 2023

gutzbenj commented Sep 29, 2023 • edited

amotl commented Sep 29, 2023 • edited

amotl commented Sep 29, 2023

kmuehlbauer commented Sep 29, 2023 • edited

kmuehlbauer commented Sep 29, 2023

amotl commented Sep 29, 2023 • edited

gutzbenj commented Sep 29, 2023

amotl Sep 29, 2023 • edited

Choose a reason for hiding this comment

amotl Sep 29, 2023 • edited

Choose a reason for hiding this comment

amotl commented Sep 29, 2023 • edited

amotl commented Sep 29, 2023 • edited

gutzbenj commented Sep 29, 2023

amotl Sep 29, 2023

Choose a reason for hiding this comment

amotl Sep 29, 2023

Choose a reason for hiding this comment

amotl Sep 29, 2023

Choose a reason for hiding this comment

amotl commented Sep 29, 2023 • edited

amotl commented Sep 29, 2023

amotl commented Sep 29, 2023

amotl Sep 29, 2023 • edited

Choose a reason for hiding this comment

amotl Sep 29, 2023 • edited

Choose a reason for hiding this comment

This comment was marked as resolved.

amotl commented Sep 29, 2023 • edited

Problem

References

Fix?

amotl commented Sep 29, 2023 • edited

amotl commented Sep 29, 2023 • edited

kmuehlbauer left a comment

Choose a reason for hiding this comment

amotl commented Sep 29, 2023

codecov bot commented Sep 29, 2023

Codecov Report

amotl commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

gutzbenj commented Sep 29, 2023 •

edited

gutzbenj commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

kmuehlbauer commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

amotl Sep 29, 2023 •

edited

amotl Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

amotl Sep 29, 2023 •

edited

amotl Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited

amotl commented Sep 29, 2023 •

edited