fix(mfsimulation): respect max_columns_of_data for external arrays #2666
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Followup on #2665
Add an optional parameter
replace_existingtoset_all_data_external()methods, defaultingTrue. This guarantees that user settings will be respected when these methods are called, at the cost of rewriting when it may not be necessary. Previously settings were only respected when writing to a new workspace. So, now:This decision prioritizes always respecting user settings. I could also see a case for defaulting
False, so you need to opt into replacing existing files if you want newly modified column settings applied. This would prioritize IO performance over respecting settings. I'm not sure what the right choice is here, curious how others feel.As mentioned in #2419, external files are written on
set_all_data_external()instead of simply configured to be external, then only written onwrite_simulation(). In other words, due to architectural quirks:This PR does nothing to address that. I have looked into a fix but it would be somewhat invasive so I'm tempted to punt to 4.x.