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

Make unique index sorted #855

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Make unique index sorted #855

merged 1 commit into from
Feb 19, 2024

Conversation

RobbeSneyders
Copy link
Member

The unique index we create is not sorted, while I believe this was originally the goal. Ran into some issues merging large dataframes and this was one of them.

Copy link
Collaborator

@GeorgesLorre GeorgesLorre left a comment

Choose a reason for hiding this comment

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

Is this something we can do/check in the parent class ?

Copy link
Contributor

@mrchtr mrchtr left a comment

Choose a reason for hiding this comment

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

Could we adjust the component test too?

@RobbeSneyders
Copy link
Member Author

Is this something we can do/check in the parent class ?

We've discussed a couple of times already to create better base components, this could indeed be part of that. Seems like we only have an issue for a base inference component (#565), but would be useful for load and write components as well.

This is just a quick fix though, would take that up as a separate effort.

Could we adjust the component test too?

Let's add a test when we move this into a generic load component.

Opened an issue for it: #860

@RobbeSneyders RobbeSneyders merged commit b0d2789 into main Feb 19, 2024
11 checks passed
@RobbeSneyders RobbeSneyders deleted the bugfix/unique-index branch February 19, 2024 13:24
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