CAPI Draft

[asvetlov]

No description provided.

[asvetlov]

The PR contains a draft for CAPI with tests.

For now, only a minimal subset of functions is exposed, just for the idea demonstration.
Cython bindings are to be done, they are built on top of C API anyway.

Tests are attached.

@Vizonex, you might be interested in the PR. It is based on your work, but it is slightly different in implementation details.

[codecov]

Codecov Report

❌ Patch coverage is 3.00752% with 129 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.32%. Comparing base (e0e61c2) to head (636f265).

Files with missing lines	Patch %	Lines
tests/test_capi.py	3.00%	129 Missing ⚠️

❌ Your patch check has failed because the patch coverage (3.00%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (95.28%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1183      +/-   ##
==========================================
- Coverage   99.85%   96.32%   -3.54%     
==========================================
  Files          26       27       +1     
  Lines        3510     3643     +133     
  Branches      252      252              
==========================================
+ Hits         3505     3509       +4     
- Misses          3      132     +129     
  Partials        2        2              

Flag	Coverage Δ
CI-GHA	96.32% <3.00%> (-3.54%)	⬇️
pytest	96.32% <3.00%> (-3.54%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

CodSpeed Performance Report

Merging #1183 will not alter performance

_{Comparing capi (636f265) with master (e0e61c2)}

Summary

✅ 245 untouched benchmarks

[webknjaz]

The full-blown packaging setup seems like huge overengineering. There's better ways to integrate with pytest that wouldn't require additional layers of complexity.

[asvetlov]

The full-blown packaging setup seems like huge overengineering. There's better ways to integrate with pytest that wouldn't require additional layers of complexity.

Sorry, I don't follow your idea.

1. We need a compiled package for testing C (and later Cython) APIs.
2. This package shouldn't be a part of multidict itself; it is required for testing only, and multidict should already be installed for compiling the test helper.

If you have a better idea, please don't hesitate to push a working code to show your thoughts.

[Vizonex]

The full-blown packaging setup seems like huge overengineering. There's better ways to integrate with pytest that wouldn't require additional layers of complexity.

Sorry, I don't follow your idea.

1. We need a compiled package for testing C (and later Cython) APIs.
2. This package shouldn't be a part of multidict itself; it is required for testing only, and multidict should already be installed for compiling the test helper.

If you have a better idea, please don't hesitate to push a working code to show your thoughts.

@asvetlov I agree with you since I spent more time than I originally anticipated trying to expose multidict's inner workings and I seem to keep having to second-guess does this go here or does this go here? At the end of the day the unpredictability of some parts of the CAPI needs a better foundation. I am willing to drop my things for a second time and just work on helping you get this passed a second time. At the end of the day. The practice I get from doing this has paid off for me in the long-run and I seem to be learning new stuff the deeper I've dived into this.

I don't even feel upset at all about dropping older pull requests since I seem to keep learning new stuff. I am actually ok with closing #1178 and moving to this pull request so that we can develop a safer implementation to use. I'll hold onto a copy of my old pull request in my folders incase I need to utilize some of it's information later.

[Vizonex]

@asvetlov I'm forking your draft instead since it has a better foundation to work upon the only thing I could see being a problem is the api calls for the capsule to be a little bit too generic since you never know if someone plans to compile this CAPI with another capsuled module. The last thing you would want is for someone to end up using the same names and colliding with it when compiling everything.

[asvetlov]

@Vizonex, your work is very valuable.
I've borrowed many ideas from your PRs, let's finish the work together.
I'll review #1185 soon, thank you for it!

[Vizonex]

@Vizonex, your work is very valuable. I've borrowed many ideas from your PRs, let's finish the work together. I'll review #1185 soon, thank you for it!

Your welcome. I seem to be learning new stuff as I am doing this.

[Vizonex]

@asvetlov Crap I just remembered I deleted my old fork I still have the code on hand and we can quickly remerge it and resume where I left off my bad. The accidental merge confused me :/

[asvetlov]

@Vizonex here is my local copy of your branch; you can use it on your own.

Sorry again; I unintentionally pushed your branch with code reformatting in the command line, but the target was my branch. GitHub processed it as a merge. git sometimes allows dangerous things :(

[Vizonex]

@asvetlov Let me write some more API Functions now that I know that you've merged my things.

[Vizonex]

@asvetlov I made a copy of my old branch and archived it to https://github.com/Vizonex/multidict/tree/vizonex-capi so that I could resync my branch with yours again. You can use it as a reference if we need to later but let me go ahead and resync with your changes first.

[Vizonex]

No Idea how we should go about implementing CIMultiDict and CIMultiDictProxy but now I have done everything else.