Migrate to paramstyle="pyformat", following crate-python 2.2.0#266
Migrate to paramstyle="pyformat", following crate-python 2.2.0#266amotl wants to merge 7 commits into
paramstyle="pyformat", following crate-python 2.2.0#266Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
amotl
force-pushed
the
paramstyle-pyformat
branch
from
fec5468 to
c58b587
Compare
|
Hi @bgunebakan. Please let me know if you think it makes sense to take over this PR, or otherwise close it without much ado at your disposal if relevant fixes are coming from upstream crate-python, so this patch might no longer be needed. Thanks! |
| fake_cursor.execute.assert_called_with( | ||
| ("UPDATE characters SET data['y'] = ?, data['x'] = ? WHERE characters.name = ?"), | ||
| (2, 1, "Trillian"), | ||
| ( | ||
| "UPDATE characters SET data['y'] = %(data_'y'_)s, data['x'] = %(data_'x'_)s " | ||
| "WHERE characters.name = %(characters_name)s" | ||
| ), | ||
| {"characters_name": "Trillian", "data_'y'_": 2, "data_'x'_": 1}, | ||
| ) |
There was a problem hiding this comment.
I don't know if this result is something users would expect. Maybe they would expect a nested representation of the data column instead (if possible at all), maybe not?
{"characters_name": "Trillian", "data_'y'_": 2, "data_'x'_": 1}
| dependencies = [ | ||
| "backports.zoneinfo<1; python_version<'3.9'", | ||
| "crate>=2,<3", | ||
| "crate>=2.2.1b1,<3", |
There was a problem hiding this comment.
While being work in progress, you can temporarily run this branch against crate head or any other branch for integration testing by using a dependency spec like this:
crate @ git+https://github.com/crate/crate-python.git@main
That can save a few cycles instead of needing to run pre-releases in lockstep while proceeding on harmonization matters.
There was a problem hiding this comment.
Thank you! We should drop support for python<3.10 because crate 2.1.0+ requires Python >=3.10. CI matrix had Python 3.7/3.8/3.9 in it. pip silently fell back to crate 2.0.0 on these jobs but there is no fallback with this branch, this can't work with older crate-python versions.
There was a problem hiding this comment.
Hi. I wanted to expand Python version compatibility because I think it is important to support people running older versions of Python despite being EOL, but it didn't land in time.
Well, I guess it's c'est la vie then; please proceed if you don't hear any other objections.
2 participants
About
A few adjustments were necessary to accommodate the test suite to upstream changes in crate-python 2.2.0. The updates show that the improvement might have an impact to downstream users, so we might want to handle it with more care even in retrospective.
References