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

Fix failing e2e #31

Merged
merged 4 commits into from Apr 7, 2023
Merged

Fix failing e2e #31

merged 4 commits into from Apr 7, 2023

Conversation

AleksanderWWW
Copy link
Contributor

@AleksanderWWW AleksanderWWW commented Apr 7, 2023

prophet plotting functionality seems to be incompatible with pandas 2.0.0 - calls to model.plot(forecast) raise ValueError: Multi-dimensional indexing (e.g. obj[:, None]) is no longer supported. Convert to a numpy array before indexing instead.

The tests pass however, with pandas versions lower than 2.0.0.

I tried applying conversion to numpy on our side, but the right place to do it is in the prophet code, not ours.
If prophet team ever update their code, I will remove those version constraints.

Also:
introduced running tests with python 3.10, since the errors this PR aims to resolve originally appeared there.

Copy link
Contributor

@twolodzko twolodzko left a comment

Choose a reason for hiding this comment

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

This happened before and our conclusion was that there was a bug in Pandas that was fixed on their side. So yes, this should fix the problems.

@@ -22,8 +22,8 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why removing Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Installation process with python3.10 and windows is a little tricky and in CI causes errors while trying to install matplotlib. Plus we don't use it in our e2e test suite triggered from the client

@AleksanderWWW AleksanderWWW merged commit 9bf8ade into main Apr 7, 2023
6 checks passed
@AleksanderWWW AleksanderWWW deleted the aw/fix-e2e branch April 7, 2023 09:25
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.

None yet

2 participants