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

More tests detecting invalid index using selfrefok #5085

Open
4 tasks
mattdowle opened this issue Jul 28, 2021 · 0 comments
Open
4 tasks

More tests detecting invalid index using selfrefok #5085

mattdowle opened this issue Jul 28, 2021 · 0 comments
Milestone

Comments

@mattdowle
Copy link
Member

mattdowle commented Jul 28, 2021

As per #5042 (comment)

  • setkey.R::getindex(). Adding the selfrefok() check there was my first attempt for don't use index when selfref detects prior copy by another package #5084 but that didn't work because shallow() in bmerge() had already set selfref to ok but left the invalid index intact, hence fixing shallow. Hopefully that was a one-off and everything does now go through getindex() in one place.
  • add verbose message(s) when indexes aren't being used due to selfref
  • [.data.table at the start so as to remove invalid indexes as soon as possible? That way, if it generates a warning optionally too, that can be generated as close as possible after the external package call that created the invalid index, to help debugging performance issues caused by indexes being dropped.
  • add tests preferably using base R like the test in setDF deletes the index attribute #4893 so they can be in the main tests.Rraw rather than other.Rraw
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

No branches or pull requests

3 participants