Skip to content

Fix for duplicate values in sparse array index#223

Merged
eddelbuettel merged 1 commit intomasterfrom
aw/allow-dupes-in-sparse-index
Apr 1, 2021
Merged

Fix for duplicate values in sparse array index#223
eddelbuettel merged 1 commit intomasterfrom
aw/allow-dupes-in-sparse-index

Conversation

@aaronwolen
Copy link
Copy Markdown
Member

fromDataFrame() was ignoring the allows_dups argument.

Copy link
Copy Markdown
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Fabulous catch. Thanks for that!

Comment thread R/DataFrame.R
tiledb_array_create(uri, schema)

allows_dups(schema) <- allows_dups
tiledb_array_create(uri, schema)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Doh. Good catch!

uri <- tempfile()
expect_silent(
arr <- fromDataFrame(df, uri, col_index=1, sparse=TRUE, allows_dups=TRUE)
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I still think that indentation style is batshit crazy. R Core has something to say in this section of R Internals but I guess nobody pays attention to them anymore 😢

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll put a quarter in the 'excessive indentation' jar 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤣

@eddelbuettel eddelbuettel merged commit af1aeb3 into master Apr 1, 2021
@eddelbuettel eddelbuettel deleted the aw/allow-dupes-in-sparse-index branch April 2, 2021 20:03
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