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

Bugfix: Arrays losing CloudArray interface #2144

Merged

Conversation

kounelisagis
Copy link
Member

@kounelisagis kounelisagis commented Jan 15, 2025

This PR fixes the case when tiledb-cloud package is present but instances of Dense and Sparse Array are returned without the correct inheritance from:

TileDB-Py/tiledb/array.py

Lines 425 to 431 in e1f57bd

return SparseArrayImpl(
uri, mode, key, timestamp, attr, ctx, preloaded_array=tmp_array
)
else:
return DenseArrayImpl(
uri, mode, key, timestamp, attr, ctx, preloaded_array=tmp_array
)
.


[sc-61938]

Copy link
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@kounelisagis Thank you! I'd like to see the test at https://github.com/TileDB-Inc/TileDB-Cloud-Py/blob/main/tests/test_threaded_import.py moved to the TileDB-Py project to guard against possible future regressions.

tiledb/array.py Outdated
CloudArray = object


class Array(CloudArray):
Copy link
Contributor

Choose a reason for hiding this comment

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

@kounelisagis Great!

@kounelisagis kounelisagis marked this pull request as draft January 15, 2025 18:14
@kounelisagis kounelisagis force-pushed the agis/simplify-inheritance-of-array-class-from-cloudarray branch from e23d31c to fee9040 Compare January 16, 2025 08:34
@kounelisagis kounelisagis changed the title Simplify inheritance of Array class from CloudArray BugFix: Arrays losing CloudArray interface Jan 16, 2025
@kounelisagis kounelisagis changed the title BugFix: Arrays losing CloudArray interface Bugfix: Arrays losing CloudArray interface Jan 16, 2025
@kounelisagis kounelisagis marked this pull request as ready for review January 16, 2025 08:44
Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

g2g as long as @sgillies agrees. thanks!

Copy link
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

Thanks @kounelisagis ! I ran tiledb-cloud's tests with this branch and the problem is resolved.

@kounelisagis kounelisagis merged commit e096e6a into main Jan 16, 2025
33 checks passed
@kounelisagis kounelisagis deleted the agis/simplify-inheritance-of-array-class-from-cloudarray branch January 16, 2025 17:07
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