Skip to content

Conversation

@kolibril13
Copy link
Owner

@BradyAJohnston
Copy link
Collaborator

I don't think we should make this a hard limit. Polars is fast enough at parsing the file that as part of the operator init we could have it parse the data, check for the number of unique strings, pop up a warning that the load will be slow to get around limitations with Blender's support for strings.

Otherwise there should be an override limit or option for the import operator for a user to override this limit. Limiting it to just using the python API I think isn't great UX.

@BradyAJohnston
Copy link
Collaborator

We should also maybe not ship test data though that takes several minutes for the average user to import

@kolibril13
Copy link
Owner Author

kolibril13 commented Mar 12, 2025

We should also maybe not ship test data though that takes several minutes for the average user to import

the problem with the test data was this first column, that provides an ID:
https://github.com/kolibril13/blender_csv_import/blob/main/docs/sample_datasets/data_california_housing.csv
image
After setting this limit of 3000 string, the test data is fast again (milliseconds instead of minutes)

What I like about this approach is that the columns that contain less than 3000 unique strings (like the labels) are imported correctly.
image

EDIT: I think the test data is actually a good example for the user, so I would prefer to not remove this example.

pop up a warning that the load will be slow to get around limitations with Blender's support for strings.

interesting! Maybe even a pop-up window that lets you select the potentially slow columns to skip?

@BradyAJohnston
Copy link
Collaborator

So the current approach just drops the larger string columns?

@kolibril13
Copy link
Owner Author

So the current approach just drops the larger string columns?

yes, it does

@BradyAJohnston
Copy link
Collaborator

I think if we promise string import and then drop certain columns the operator should report a warning after importing.

@kolibril13
Copy link
Owner Author

good point! Here's now the updated warning:

            else:
                warning_message = f"Column '{col}' has {len(unique)} unique strings, which exceeds the limit of {string_limit}. This column will be skipped. You can increase the limit with the string_limit parameter."
                warnings.warn(warning_message)
                self = bpy.context.window_manager
                self.popup_menu(lambda self, context: self.layout.label(text=warning_message), title="Warning", icon='ERROR')

image

@kolibril13
Copy link
Owner Author

thanks for the review!

@kolibril13 kolibril13 merged commit 102060a into main Mar 13, 2025
12 checks passed
@kolibril13 kolibril13 deleted the string_limit branch March 13, 2025 09:20
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.

Example for custom_string_iswitch

3 participants