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

Fix: Correct file reading logic for single row/column detection #602

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gvaleras
Copy link

The check array.shape == 1 cannot distinguish between a single row and a single column. Using if max_rows == 1 correctly resolves this ambiguity, ensuring the array is reshaped as expected.

@gvaleras
Copy link
Author

Hello @kinverarity1, could you please review this PR when possible? It needs to be rebased with the latest main, and I noticed that python3.7 (which caused the CI failure) and python3.8 have been dropped from main since then. Additionally, could you please approve the workflow so it can run? Thank you!

@kinverarity1 kinverarity1 marked this pull request as draft March 23, 2025 08:03
@kinverarity1 kinverarity1 marked this pull request as ready for review March 23, 2025 08:03
@kinverarity1
Copy link
Owner

kinverarity1 commented Mar 23, 2025

Sorry the review button has disappeared. Could you change the style to avoid in-line if?
e.g.

if max_rows == 1:
    array = array.reshape(arr_len, 1)
else:
    array = array.reshape(1, arr_len)

Maybe a new PR would be good so I can merge it without having to rebase?

@kinverarity1
Copy link
Owner

Thank you for working on it

…d a single column.

Using if max_rows == 1 correctly resolves this ambiguity, ensuring the array is reshaped as expected.
@gvaleras gvaleras force-pushed the bugfix/file-read-shape-mismatch-when-single-column branch from c1d151a to 8dc44e8 Compare March 23, 2025 20:02
@gvaleras
Copy link
Author

Hello @kinverarity1, I have rebased the branch with main and applied your requested changes, replacing the inline if with a block if-else. Please have a look and let me know if anything else is needed. Thank you!

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.

2 participants