Alwaysgaurav1 commented on PR #69553: URL: https://github.com/apache/airflow/pull/69553#issuecomment-4921737424
> Nice fix, and it holds up: checked against `seq[start:stop:step]`, the crossed-bound cases that used to produce a negative `LIMIT` now come back as `[]`. > > A couple of things worth considering: > > 1. A regression test would be great here. `test_xcom_get_with_slice` already compares against `["f", "o", "b"][key]`, so a few crossed-bound params (`slice(2, 1)` for the `start >= 0` branch, `slice(-2, 1)` for the `start < 0` branch) would pin it down and fail without the guards. > 2. The same `start < 0` branch still returns the wrong window when `stop` overshoots the end: on a 5-item list, `seq[-2:6]` gives `['c', 'd', 'e']` instead of `['d', 'e']` (off by one, and present on main too, so not introduced here). It could be worth folding into this PR given the overlap Thanks for noticing this, and I just fixed that change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
