Use can_ada for supported URLs#261
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #261 +/- ##
==========================================
- Coverage 98.70% 98.39% -0.31%
==========================================
Files 9 9
Lines 848 872 +24
Branches 172 175 +3
==========================================
+ Hits 837 858 +21
- Misses 4 9 +5
+ Partials 7 5 -2
🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 22.42%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_benchmark_url_cold[parse_url] |
145.9 µs | 111.5 µs | +30.85% |
| ⚡ | test_benchmark_url_cold[add_or_replace_parameter] |
746.9 µs | 574 µs | +30.12% |
| ⚡ | test_benchmark_url_cold[safe_url_string] |
1.9 ms | 1.5 ms | +24.94% |
| ⚡ | test_benchmark_url_cold[safe_download_url] |
392.5 µs | 318.2 µs | +23.36% |
| ⚡ | test_benchmark_url_cold[add_or_replace_parameters] |
328.4 µs | 287.8 µs | +14.08% |
| ⚡ | test_benchmark_url_cold[canonicalize_url] |
350.2 µs | 311.4 µs | +12.45% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing AdrianAtZyte:ada (38f52bf) with master (d5877fe)
| ) | ||
| assert isinstance(safeurl, str) | ||
| assert safeurl == "http://www.example.com/%C2%A3?unit=%B5" | ||
| assert safeurl == "http://www.example.com/%C2%A3?unit=%C2%B5" |
There was a problem hiding this comment.
There are several changes that look like it was latin1 and now is utf-8, and in at least some of them an arg with "latin1" is passed to the function, I wonder if all of these are still valid behavior.
There was a problem hiding this comment.
My understanding is that latin1 is used for decoding bytes to str, then utf-8 for percent-encoding, which I think aligns with WHATWG.
Yes, it says that explicitly so maybe we don't want to have those tests... |
#221 (comment)
Fixes #98, fixes #204, closes #221, fixes #222, closes #270.
Some of the tests in #259 would still not pass with these changes, but I suspect that may be OK (can_ada / WHATWG diverge from stdlib).