Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore/upgrade dependencies #188

Merged
merged 23 commits into from
Sep 20, 2024
Merged

Chore/upgrade dependencies #188

merged 23 commits into from
Sep 20, 2024

Conversation

Flix6x
Copy link
Collaborator

@Flix6x Flix6x commented Sep 19, 2024

Improve dependency management, and unpin some requirements for optional dependencies.

Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
…everything with share dependencies

Signed-off-by: F.N. Claessen <[email protected]>
@Flix6x Flix6x added the dependencies Pull requests that update a dependency file label Sep 19, 2024
@Flix6x Flix6x self-assigned this Sep 19, 2024
@Flix6x Flix6x added this to the 3.1.0 milestone Sep 19, 2024
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the 3.11 test, I see many errors saying " KeyError: "DataFrame should contain column named 'source'.""

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no dev/3.10/requirements.txt, dev/3.8/requirements.txt or dev/3.7/requirements.txt

Maybe I am overlooking something but why do the tests for these Python versions not fail on that?

@nhoening
Copy link
Contributor

Mh, #177 is already closed?
Why would this PR close it again?
But I see its error message pop up in the 3.11 test ....

@Flix6x
Copy link
Collaborator Author

Flix6x commented Sep 19, 2024

There are no dev/3.10/requirements.txt, dev/3.8/requirements.txt or dev/3.7/requirements.txt

Maybe I am overlooking something but why do the tests for these Python versions not fail on that?

The pipeline doesn't install the dev requirements, but decides what to install based on the pyproject.toml.

@Flix6x
Copy link
Collaborator Author

Flix6x commented Sep 19, 2024

Mh, #177 is already closed? Why would this PR close it again? But I see its error message pop up in the 3.11 test ....

I had closed it by addressing the issue in #178, which pinned Pandas. I thought I could unpin it in this PR, but alas.

It turns out that for some reason my local Python 3.11 tests run different dependency versions than the GitHub pipeline. I'm still not sure why. The pipeline just runs make test, which I'm also doing locally.

FYI, I had asked you to review just before I realized that the pipeline wasn't checking 3.11 yet. After I added 3.11 to the pipeline, I realized that Pandas still needs a pin. Tests now pass, but unfortunately, pinning Pandas also means numpy is not going beyond 1.26.4 for a timely-beliefs installation that uses the optional [forecast] dependencies (with pip install timely-beliefs[forecast]).

@Flix6x Flix6x mentioned this pull request Sep 19, 2024
@nhoening
Copy link
Contributor

Pandas needs a on only for 3.11?

@Flix6x
Copy link
Collaborator Author

Flix6x commented Sep 19, 2024

Pandas needs a on only for 3.11?

What?

@nhoening
Copy link
Contributor

Sorry. Is the pin only needed for 3.11?

@Flix6x
Copy link
Collaborator Author

Flix6x commented Sep 19, 2024

Sorry. Is the pin only needed for 3.11?

Ah I see, because only that version failed the tests. Well, I could test for 3.12, too, perhaps. And for <3.11 it seems we could let the pin loose indeed.

@Flix6x
Copy link
Collaborator Author

Flix6x commented Sep 19, 2024

Sorry, it doesn't look like I can let go the pin on pandas<2.2.2.

@nhoening
Copy link
Contributor

Well, at least that is very recent.
numpy sits at a version from 7 months ago (just under version 2), why is that? I don't see a pin for it? maybe that comes via pandas?

@nhoening
Copy link
Contributor

Oh yeah, pandas 2.2.1 is from February, and even 2.2.2 is from April

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not update the 3.9 requirements? It seems it uses vastly different versions (e.g. looking at pandas)

@Flix6x
Copy link
Collaborator Author

Flix6x commented Sep 19, 2024

Pandas 2.2.2 is the first one compatible with numpy 2, but it breaks metadata propagation for our subclasses.

Updating dev requirements is not meant to be part of this PR.

@Flix6x Flix6x merged commit 62e1a76 into main Sep 20, 2024
7 checks passed
@Flix6x Flix6x deleted the chore/upgrade-dependencies branch September 20, 2024 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants