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

[dims] previously non-consecutive dims could get lost #1015

Merged
merged 1 commit into from
May 9, 2024
Merged

Conversation

JanMarvin
Copy link
Owner

@JanMarvin JanMarvin commented May 9, 2024

if rows and columns were entirely omitted. This could create a cc object that was to short. fixes #1014

…lumns were entirely omitted. This could create a cc object that was to short. fixes #1014
@JanMarvin
Copy link
Owner Author

It's not the proudest thing I've ever programmed, but it should be better than the previous approach.

The problem is that the length of the dims of the data to be selected and the entire data range are unequal.
Imagine a case where dims skips a row, as in “A1:B2,A4:B6”. In some places, we simply loop over the number of columns and rows. If a row is missing, our loops are not aware of it. There is nothing to do for row 3, but they iterate over it and are at n+1 and therefore later stop at n-1. The correction therefore enlarges the selected data frame so that it covers the entire data range. This way, when looping over the data, both sizes are equal.

Unfortunately, in these cases we might have to create a few empty columns and rows.

The whole code is unfortunately a little ... confusing. I'm not going to call it spaghetti code just yet, but it's getting close due to the various stages of development we're still supporting. One day write_data() and write_data2() should be combined and the whole thing tidied up. We can also iterate over the necessary rows and columns. But that will probably only happen in the distant future.

@JanMarvin JanMarvin merged commit 01f91de into main May 9, 2024
9 checks passed
@JanMarvin JanMarvin deleted the gh_issue_1014 branch May 9, 2024 15:04
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.

Non-Consecutive dim gets lost
1 participant