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/Generalize test added in #2083 #2116

Merged
merged 1 commit into from
Jan 3, 2025
Merged

Fix/Generalize test added in #2083 #2116

merged 1 commit into from
Jan 3, 2025

Conversation

kounelisagis
Copy link
Member

@kounelisagis kounelisagis commented Nov 29, 2024

In #2083, the "S" dtype is used in the new test to verify that dropping a fixed-size attribute and adding it back as variable-size now works as expected. However, this caused the issue described here: TileDB-Inc/conda-forge-nightly-controller#151 (comment).

Instead of testing with a specific value, we should test with both "S" and "U" dtypes but assert the fill value of the attribute before writing any data to it. This approach makes the test more generic for all types.

The same problem also appears in #2114, so it would be better to apply this fix before merging that PR.

cc @jdblischak, @dudoslav


[sc-59851]

@kounelisagis kounelisagis marked this pull request as ready for review November 29, 2024 13:56
@kounelisagis kounelisagis changed the title Alter type used in test added in #2083 Fix/Generalize test added in #2083 Dec 12, 2024
@jdblischak
Copy link
Contributor

The only CI failure was for macOS with Python 3.9: ERROR at teardown of TestDaskSupport.test_dask_multiattr_2d

I suspect this was a spurious failure, and regardless appears unrelated to the changes this PR makes to tiledb/tests/test_schema_evolution.py. I'd restart the failed job but I don't have sufficient priveleges.

Copy link
Contributor

@jdblischak jdblischak left a comment

Choose a reason for hiding this comment

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

This test failure is blocking the feedstock release of 0.33.0 in conda-forge/tiledb-py-feedstock#249

@kounelisagis kounelisagis merged commit 600d374 into main Jan 3, 2025
33 checks passed
@kounelisagis kounelisagis deleted the agis/test-fix branch January 3, 2025 15:21
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.

3 participants