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

BUG: Remove Python 3.8 conditions in test.yml #96

Merged
merged 2 commits into from
Nov 8, 2024
Merged

Conversation

thewtex
Copy link
Contributor

@thewtex thewtex commented Nov 5, 2024

We no longer support Python 3.8

@thewtex thewtex requested a review from melonora November 5, 2024 20:13
@thewtex thewtex force-pushed the test-yaml-python-3-8 branch from b46fcb4 to cf5df10 Compare November 5, 2024 20:29
@melonora
Copy link
Collaborator

melonora commented Nov 6, 2024

will test this

@melonora
Copy link
Collaborator

melonora commented Nov 6, 2024

I had to drop the python 3.13 support first as this was causing issues (no xarray-dataclasses compatible in python 3.13) that were previously not caught due to the if condition.

@melonora
Copy link
Collaborator

melonora commented Nov 6, 2024

For later we might decide to downscale the CI a bit?

We no longer support Python 3.8
@thewtex thewtex force-pushed the test-yaml-python-3-8 branch from d859918 to 163c50b Compare November 8, 2024 14:22
@thewtex
Copy link
Contributor Author

thewtex commented Nov 8, 2024

OK, the tests are running again.

Windows, Linux, Intel mac are passing.

For ARM mac, we get some errors:

E           AssertionError: Left and right DatasetView objects are not equal
63
E           Differing data variables:
64
E           L   cthead1  (y, x) uint8 16kB 0 0 0 0 0 0 0 0 0 0 0 0 ... 0 0 0 0 0 0 0 0 0 0 0
65
E           R   cthead1  (y, x) uint8 16kB dask.array<chunksize=(128, 128), 

@melonora I will not have to dive into this until late next week if you want to take a look we could skip ARM mac for now.

@melonora
Copy link
Collaborator

melonora commented Nov 8, 2024

I will try to enforce a compute for now as it seems one is the lazy representation and the other one in memory. I have seen similar things with macs before that there are different behaviours for dask across mac versions.

@melonora
Copy link
Collaborator

melonora commented Nov 8, 2024

oof forcing a compute gives this:
image
The values are actually different also on other platforms so I am not certain whether the comparison checks are actually working.

@melonora
Copy link
Collaborator

melonora commented Nov 8, 2024

I will check whether tests pass with spatialdata if they do, I will revert last commit and skip MAC arm and open an issue, but then still release. Alright with you @thewtex?

@melonora
Copy link
Collaborator

melonora commented Nov 8, 2024

tests in spatialdata pass

@melonora
Copy link
Collaborator

melonora commented Nov 8, 2024

so I suspect still an issue with the test data

Baseline test comparisons are failing, tracked in #100.
@thewtex thewtex mentioned this pull request Nov 8, 2024
@thewtex thewtex force-pushed the test-yaml-python-3-8 branch from 0f0a80d to 189f985 Compare November 8, 2024 15:30
@thewtex
Copy link
Contributor Author

thewtex commented Nov 8, 2024

@melonora, I disabled it in CI and opened an issue in #100 .

We can move forward with additional fixes on top of this.

@thewtex thewtex merged commit f3f6c03 into main Nov 8, 2024
21 checks passed
@thewtex thewtex deleted the test-yaml-python-3-8 branch November 8, 2024 15:32
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.

2 participants