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

Add maintain_size to keep asked for size in get, get_values #1305

Merged
merged 10 commits into from
Oct 21, 2023

Conversation

alifeee
Copy link
Collaborator

@alifeee alifeee commented Sep 28, 2023

closes #1289

Users can now specify maintain_size=True, so that "A1:C3" will always be a 3x3 array, regardless of whether the cells are full or not. See #1289 more more explanation.

>>> worksheet.get_values("A1:D4")
[
    ["1", "2"],
    ["3", "4"],
    ["5", "6"],
]
>>> worksheet.get_values("A1:D4", maintain_size=True)
[
    ["1", "2", "", ""],
    ["3", "4", "", ""],
    ["5", "6", "", ""],
    ["",  "",  "", ""],
]

In the future (6.0.0), this could be changed to be default behaviour. Thoughts?

"A1:B2" -> True
"Sheet1!A1:B2" -> True
"A1:B" -> False
"4:7" -> False
basically removes "!" from range
"Sheet1!A1:B2" -> "A1:B2"
"A1:B2" -> "A1:B2"
@lavigne958
Copy link
Collaborator

Hi I'll look at the code when I get a chance.

About the default behavior I'm not too sure. This might be a big thing or not. I have no clue what users expect and if that's gonna break something.

We can introduce it. Introduce all the breaking change we have first then maybe, do a release 7.0.0 not too far later with this kind of breaking change. What do you think ?

@alifeee
Copy link
Collaborator Author

alifeee commented Oct 7, 2023

What do you think ?

I'm not sure. I don't have a good enough understanding of who uses gspread and how they use it to say whether this would have a large or small affect on people.

I think: people could call "A1:C4" and get a range that is not 3x4, and then be confused. They could then either:

  • make their own function to pad the values
  • read the gspread documentation and find "maintain_size"

I think the latter is less likely, which is why I suggested to make it default behaviour. If people did option 1 above, then their code would break, but they would just have to remove their padding function becase we provide it natively.

gspread/utils.py Outdated Show resolved Hide resolved
gspread/worksheet.py Show resolved Hide resolved
gspread/worksheet.py Outdated Show resolved Hide resolved
@lavigne958
Copy link
Collaborator

What do you think ?

I'm not sure. I don't have a good enough understanding of who uses gspread and how they use it to say whether this would have a large or small affect on people.

I think: people could call "A1:C4" and get a range that is not 3x4, and then be confused. They could then either:

  • make their own function to pad the values
  • read the gspread documentation and find "maintain_size"

I think the latter is less likely, which is why I suggested to make it default behaviour. If people did option 1 above, then their code would break, but they would just have to remove their padding function becase we provide it natively.

I agree, I would expect it too, the size of what I request to match.

What I am afraid of is if users have some kind of code around that, would it break ?

for sure the new parameter allows users to change using it for now, and then next major release we make default and then boom it's automatic.

@alifeee
Copy link
Collaborator Author

alifeee commented Oct 17, 2023

This line of the CI fails now

- run: shopt -s globstar && pyupgrade --py3-only **/*.py # --py36-plus

Not sure why...

(cherry picked from commit 4de778e)
@alifeee
Copy link
Collaborator Author

alifeee commented Oct 19, 2023

CI fixed with #1319. Now ready for review

@alifeee alifeee requested a review from lavigne958 October 19, 2023 09:37
Copy link
Collaborator

@lavigne958 lavigne958 left a comment

Choose a reason for hiding this comment

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

In order for the users to notice the new parameter we should probably:

  • update the examples on the readme.md and use it about everywhere ?
  • add a mention in the readme.md about the new argument ?

@alifeee
Copy link
Collaborator Author

alifeee commented Oct 20, 2023

Yes, that sounds good to me. I will be able to do it from next week.

@lavigne958 lavigne958 merged commit 2991e46 into master Oct 21, 2023
@lavigne958 lavigne958 deleted the feature/maintain-input-size branch October 21, 2023 16:20
@alifeee alifeee mentioned this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

size of range returned by get and get_values can be smaller than the input size
2 participants