[VitorCoelho]

What

GetEntitySetFilterChainable translated the __range lookup into the operators gte / lte:

service.entity_sets.Employees.get_entities().filter(ID__range=(20, 50))
# before: $filter=ID gte 20 and ID lte 50
# after:  $filter=ID ge 20 and ID le 50

Why

OData has no gte / lte operators. The comparison operators are eq, ne, gt, ge, lt, le (OData v2 URI conventions, 4.5 Filter System Query Option), so a service rejects the generated $filter with a syntax error. The __gte / __lte lookups already emit ge / le; __range now does the same.

How it was tested

• The three existing tests that cover __range asserted the invalid URL. I updated them to the correct one: they fail on master and pass with the fix.
• pytest: 298 passed.
• CHANGELOG updated (Unreleased / Fixed).

[phanak-sap]

Hi @VitorCoelho thanks for the PR. I have been curious how something that is not Odata protocol ended in - and it seems it is from #113.

If I understood the change correctly, we have the "gte/lte" wrong from Odata filter expression, but tailored for Django filtering - so for better or worse, already six years this is a stable feature in pyodata for anybody who uses pyodata + django together. Direct merging of this PR as is would be a breaking change for that - that would not be nice to debug on that code side.

I agree we should be adhering to odata specification first and generate "$filter=ID ge 20 and ID le 50". But I think we need to think about supporting current usage path as well and handle this change somehow more gracefully. To support both you - wanting to use it as "Odata should be" and those who simply still "passed Django filters built elsewhere".

[phanak-sap]

I am converting to draft directly due that, so even when all build passes this is not merged accidently at the moment.

[VitorCoelho]

@phanak-sap thanks for looking into the history, that makes sense, and I agree a silent change of the generated URL would be unpleasant for anyone relying on it.

One thing I noticed while tracing it: in the same change (#113) the single lookups __gte / __lte already emit ge / le, so only __range produces gte / lte. That looks like an inconsistency rather than intended Django output, but I fully agree existing users should not be broken by it.

Would one of these work for you?

1. Add an option to pyodata.v2.model.Config (e.g. range_filter_operators='legacy'|'odata'), keep gte/lte as the default for now and emit a DeprecationWarning when __range is used with the legacy setting, then switch the default in the next major version.
2. Or, same option, but defaults to the OData-compliant ge/le, with the legacy value available for users who need the old URLs, plus a CHANGELOG note.

I'll be happy to implement your preference, with tests for both settings.

[phanak-sap]

@phanak-sap thanks for looking into the history, that makes sense, and I agree a silent change of the generated URL would be unpleasant for anyone relying on it.

One thing I noticed while tracing it: in the same change (#113) the single lookups __gte / __lte already emit ge / le, so only __range produces gte / lte. That looks like an inconsistency rather than intended Django output, but I fully agree existing users should not be broken by it.

Would one of these work for you?

1. Add an option to pyodata.v2.model.Config (e.g. range_filter_operators='legacy'|'odata'), keep gte/lte as the default for now and emit a DeprecationWarning when __range is used with the legacy setting, then switch the default in the next major version.
2. Or, same option, but defaults to the OData-compliant ge/le, with the legacy value available for users who need the old URLs, plus a CHANGELOG note.

I'll be happy to implement your preference, with tests for both settings.

I did not have time to read properly the code, just noted the history and PR #113.
Seeing now the _build_expression it seems I was worried about the wrong thing.

Yes, 'lte' and' gte' filter expression creates correct URL snippet, while range not. I see it in current HEAD.

Your fix is therefore correct, just nobody reported a bug yet (not using "range" at all it seems). If the URL is rejected by server since not OData compliant, that's a valid bugfix and no Config nor deprecation warning is needed.

On other hand, that also means that Odata VALID 'le' and 'ge' operators are not directly usable as filter expression, just the Django flavor style. I would therefore simply add them as additional case: lines so user could filter directly based on OData specification without any Django knowledge. With line comment why there is something Odata incompliant.

It could be then thought later how to split "Odata only" filtering from "Django style" and from "hypothetical third ORM filtering flavor", if that need arises.

Does that make sense to you? Feel free to point out problem directly.

[VitorCoelho]

Thanks, that makes perfect sense, and good to know the Config option isn't needed.

I added __le / __ge as extra cases next to __lte / __gte, with a comment on why both names exist, plus a test for each and a CHANGELOG entry under Added. Full suite: 300 passed on Python 3.10 and 3.13.

Please feel free to mark it ready for review when you're happy with it. I'll stop bothering you on plain Sunday :P

[phanak-sap]

Thanks, that makes perfect sense, and good to know the Config option isn't needed.

I added __le / __ge as extra cases next to __lte / __gte, with a comment on why both names exist, plus a test for each and a CHANGELOG entry under Added. Full suite: 300 passed on Python 3.10 and 3.13.

Please feel free to mark it ready for review when you're happy with it. I'll stop bothering you on plain Sunday :P

no problem, no bothering. It is up to me when I check PRs :D

[phanak-sap]

Thanks, that makes perfect sense, and good to know the Config option isn't needed.

I added __le / __ge as extra cases next to __lte / __gte, with a comment on why both names exist, plus a test for each and a CHANGELOG entry under Added. Full suite: 300 passed on Python 3.10 and 3.13.

Please feel free to mark it ready for review when you're happy with it. I'll stop bothering you on plain Sunday :P

Looks good on the quick pass now, new relevant tests added, existing modified just for the fix part, I will look for sure tomorrow after sleep just to be extra sure, but this is heading to be part of next bugfix version.

[phanak-sap]

@VitorCoelho Huh? Its gone? This was not yet merged..