Implementation of `maybe_single`

[bariqhibat]

trafficstars

Signed-off-by: Bariq [email protected] Resolve #94

Note:

1. This is still under development, might need some help and comments on this (cc @dreinon)

[anand2312]

Thanks for the PR!

Tests are failing with some odd duplicated validator pydantic error that I can't figure out the reason for 😦

I understand that this is the implementation that the JS client has, but I've been thinking lately about other ways of implementing the single (and by extension maybe_single) methods: Create new RequestBuilder classes, maybe SingleRequestBuilder/MaybeSingleRequestBuilder, and then modify the execute method on those builder classes instead of monkey-patching it in like it is done here. Pseudocode:

class SelectRequestBuilder:
    def limit(n: int) -> QueryRequestBuilder:
        ... # this returns the ordinary query request builder, whose `execute` method will return an APIResponse
    def single() -> SingleRequestBuilder:
        ... # the execute method on this request builder will return a SingleAPIResponse
    def maybe_single() -> MaybeSingleRequestBuilder:
        ...

class MaybeSingleRequestBuilder:
    def execute() -> Optional[SingleAPIResponse]:
        # the execute logic that is used in this PR (__maybe_single_wrapper)

(The difference between APIResponse and SingleAPIResponse would be that APIResponse.data would be typed as list[dict[str, Any]] while SingleAPIResponse.data would be dict[str, Any])

In my opinion this would be better because it avoids the monkey-patching logic, which can be a little more confusing to understand at first glance. This would also be more type-safe (users would be able to type check their code with mypy/pyright, but these tools cannot statically check types correctly in places where we dynamically replace methods)

What do you think? @dreinon @bariqhibat

[bariqhibat]

Hey @anand2312,

Thanks for the reply! Definitely looks more readable and worth doing, I'll try to fit the changes here.

[anand2312]

Thanks! Feel free to ask any questions you have here, or ping me on Discord in the supabase server https://discord.gg/8ZhnE9SF (my username there is also anand)

[dreinon]

Thanks for the PR!

Tests are failing with some odd duplicated validator pydantic error that I can't figure out the reason for frowning

I understand that this is the implementation that the JS client has, but I've been thinking lately about other ways of implementing the single (and by extension maybe_single) methods: Create new RequestBuilder classes, maybe SingleRequestBuilder/MaybeSingleRequestBuilder, and then modify the execute method on those builder classes instead of monkey-patching it in like it is done here. Pseudocode:

class SelectRequestBuilder:
    def limit(n: int) -> QueryRequestBuilder:
        ... # this returns the ordinary query request builder, whose `execute` method will return an APIResponse
    def single() -> SingleRequestBuilder:
        ... # the execute method on this request builder will return a SingleAPIResponse
    def maybe_single() -> MaybeSingleRequestBuilder:
        ...

class MaybeSingleRequestBuilder:
    def execute() -> Optional[SingleAPIResponse]:
        # the execute logic that is used in this PR (__maybe_single_wrapper)

(The difference between APIResponse and SingleAPIResponse would be that APIResponse.data would be typed as list[dict[str, Any]] while SingleAPIResponse.data would be dict[str, Any])

In my opinion this would be better because it avoids the monkey-patching logic, which can be a little more confusing to understand at first glance. This would also be more type-safe (users would be able to type check their code with mypy/pyright, but these tools cannot statically check types correctly in places where we dynamically replace methods)

What do you think? @dreinon @bariqhibat

Yeah! I agree, good idea! @anand2312 @bariqhibat

[bariqhibat]

(A bit late) Eid Mubarak! Thanks for making the changes, I have some questions though.

Also, we use unasync to automatically generate the sync client, which means that changes only need to be made on the async side, and then we run unasync to generate the equivalent sync stuff. Should've documented that somewhere better, sorry 😅

Thank you! Got it on the unasync part. Just saw your reviews and replied to it.

[codecov[bot]]

Codecov Report

Merging #118 (b0d7d0f) into master (18334f8) will decrease coverage by 0.80%. The diff coverage is 84.33%.

@@            Coverage Diff             @@
##           master     #118      +/-   ##
==========================================
- Coverage   92.78%   91.97%   -0.81%     
==========================================
  Files          23       23              
  Lines         942     1097     +155     
==========================================
+ Hits          874     1009     +135     
- Misses         68       88      +20     

Impacted Files	Coverage Δ
postgrest/_sync/client.py	90.00% <ø> (ø)
postgrest/base_request_builder.py	78.36% <68.42%> (-0.91%)	:arrow_down:
postgrest/_async/request_builder.py	84.52% <72.22%> (-11.40%)	:arrow_down:
postgrest/_sync/request_builder.py	84.52% <72.22%> (-11.40%)	:arrow_down:
postgrest/__init__.py	100.00% <100.00%> (ø)
tests/_async/test_client.py	100.00% <100.00%> (ø)
tests/_async/test_request_builder.py	100.00% <100.00%> (ø)
tests/_sync/test_client.py	100.00% <100.00%> (ø)
tests/_sync/test_request_builder.py	100.00% <100.00%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f30e688...b0d7d0f. Read the comment docs.

[sourcery-ai[bot]]

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 1.69%.

Quality metrics	Before	After	Change
Complexity	0.38 ⭐	0.62 ⭐	0.24 👎
Method Length	25.16 ⭐	27.99 ⭐	2.83 👎
Working memory	5.14 ⭐	5.40 ⭐	0.26 👎
Quality	90.51% ⭐	88.82% ⭐	-1.69% 👎

Other metrics	Before	After	Change
Lines	1459	1818	359

Changed files	Quality Before	Quality After	Quality Change
postgrest/base_request_builder.py	87.42% ⭐	87.00% ⭐	-0.42% 👎
postgrest/_async/request_builder.py	84.14% ⭐	82.89% ⭐	-1.25% 👎
postgrest/_sync/client.py	96.01% ⭐	96.01% ⭐	0.00%
postgrest/_sync/request_builder.py	84.16% ⭐	82.94% ⭐	-1.22% 👎
tests/_async/test_client.py	92.39% ⭐	87.17% ⭐	-5.22% 👎
tests/_async/test_request_builder.py	94.09% ⭐	93.81% ⭐	-0.28% 👎
tests/_sync/test_client.py	92.41% ⭐	87.24% ⭐	-5.17% 👎
tests/_sync/test_request_builder.py	94.09% ⭐	93.81% ⭐	-0.28% 👎

Here are some functions in these files that still need a tune-up:

File	Function	Complexity	Length	Working Memory	Quality	Recommendation
tests/_async/test_request_builder.py	request_response_with_data	0 ⭐	31 ⭐	10 😞	79.58% ⭐	Extract out complex expressions
tests/_async/test_request_builder.py	request_response_with_single_data	0 ⭐	31 ⭐	10 😞	79.58% ⭐	Extract out complex expressions
tests/_sync/test_request_builder.py	request_response_with_data	0 ⭐	31 ⭐	10 😞	79.58% ⭐	Extract out complex expressions
tests/_sync/test_request_builder.py	request_response_with_single_data	0 ⭐	31 ⭐	10 😞	79.58% ⭐	Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

• ⭐ excellent
• 🙂 good
• 😞 poor
• ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.

---

Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

[J0]

Going to take the liberty to merge this since tests are passing and it has gone through a few rounds of review.

Let me know if there's anything else that should be fixed though - can file follow up PRs.