Skip to content

Use can_ada for supported URLs#261

Open
AdrianAtZyte wants to merge 5 commits into
scrapy:masterfrom
AdrianAtZyte:ada
Open

Use can_ada for supported URLs#261
AdrianAtZyte wants to merge 5 commits into
scrapy:masterfrom
AdrianAtZyte:ada

Conversation

@AdrianAtZyte

@AdrianAtZyte AdrianAtZyte commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

#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).

@AdrianAtZyte AdrianAtZyte requested a review from wRAR June 10, 2026 09:50
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (d5877fe) to head (38f52bf).

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     
Files with missing lines Coverage Δ
w3lib/_url.py 98.98% <100.00%> (+0.60%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq

codspeed-hq Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 22.42%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 6 improved benchmarks
✅ 39 untouched benchmarks

Performance Changes

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)

Open in CodSpeed

Comment thread tests/test_html.py Outdated
Comment thread tests/test_url.py
)
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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that latin1 is used for decoding bytes to str, then utf-8 for percent-encoding, which I think aligns with WHATWG.

@wRAR

wRAR commented Jun 10, 2026

Copy link
Copy Markdown
Member

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).

Yes, it says that explicitly so maybe we don't want to have those tests...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants