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

Merge master into feature/release_6_0_0 #1320

Merged
merged 87 commits into from
Oct 26, 2023

Conversation

alifeee
Copy link
Collaborator

@alifeee alifeee commented Oct 19, 2023

Done apart from one test fails. Not sure why.

alifeee and others added 30 commits August 17, 2023 12:01
remove CellNotFound
return SpreadsheetNotFound if spreadsheet not found
(before it was returning API error, see #1244)
Add deprecation warnings for colors
Add better Exceptions on opening spreadsheets
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
…ns/checkout-4

Bump actions/checkout from 3 to 4
remove Drive API access on Spreadsheet init (FIX - VERSION 5.11.1)
Fix merge_combined_cells in get_values (AND 5.11.2 RELEASE)
@alifeee
Copy link
Collaborator Author

alifeee commented Oct 19, 2023

This test fails

@pytest.mark.vcr()
def test_get_values_merge_cells_outside_of_range(self):
self.sheet.resize(4, 4)
sheet_data = [
["1", "2", "4", ""],
["down", "", "", ""],
["", "", "2", ""],
["num", "val", "", "0"],
]
self.sheet.update("A1:D4", sheet_data)
self.sheet.merge_cells("A2:A3")
self.sheet.merge_cells("C1:D2")
REQUEST_RANGE = "A1:B2"
expected_values = [
["1", "2"],
["down", ""],
]
values_with_merged = self.sheet.get_values(
REQUEST_RANGE, combine_merged_cells=True
)
self.assertEqual(values_with_merged, expected_values)

With the failure:

        if response.ok:
            return response
        else:
>           raise APIError(response)
E           gspread.exceptions.APIError: {'code': 400, 'message': 'Invalid value 
at \'data.values\' (type.googleapis.com/google.protobuf.ListValue), "A1:D4"', 'status': 'INVALID_ARGUMENT', 'details': [{'@type': 'type.googleapis.com/google.rpc.BadRequest', 'fieldViolations': [{'field': 'data.values', 'description': 'Invalid value at \'data.values\' (type.googleapis.com/google.protobuf.ListValue), "A1:D4"'}]}]}

Looks like setting the initial data isn't working.

Ok, I realised this was because update switched values. This was hard to spot. Maybe we should catch this in v6 and raise a GspreadException if the arguments are the wrong way round

@alifeee alifeee requested a review from lavigne958 October 19, 2023 11:12
@alifeee alifeee self-assigned this Oct 19, 2023
@alifeee alifeee added this to the 6.0.0 milestone Oct 19, 2023
@alifeee
Copy link
Collaborator Author

alifeee commented Oct 19, 2023

I have merged master into feature/release_6_0_0

Pending @lavigne958's review. See changed files.

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

Ok, I realised this was because update switched values. This was hard to spot. Maybe we should catch this in v6 and raise a GspreadException if the arguments are the wrong way round

hum true that 🤔 do we have a way to validate that ? something like checking the type of the values may be ?

@alifeee
Copy link
Collaborator Author

alifeee commented Oct 19, 2023

hum true that 🤔 do we have a way to validate that ? something like checking the type of the values may be ?

I think we can do that. Could you create another issue to track it?

gspread/worksheet.py Outdated Show resolved Hide resolved
Co-authored-by: Alexandre Lavigne <[email protected]>
gspread/worksheet.py Outdated Show resolved Hide resolved
gspread/worksheet.py Outdated Show resolved Hide resolved
@alifeee alifeee requested a review from lavigne958 October 25, 2023 15:29
@alifeee alifeee merged commit ba7bfc6 into feature/release_6_0_0 Oct 26, 2023
10 checks passed
@alifeee alifeee deleted the feature/release_6_0_0-updated branch October 26, 2023 12:40
@alifeee alifeee mentioned this pull request Jan 26, 2024
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.

5 participants