Skip to content

Add module expressions#192

Merged
seddonym merged 1 commit intomasterfrom
add-module-expressions
Mar 6, 2025
Merged

Add module expressions#192
seddonym merged 1 commit intomasterfrom
add-module-expressions

Conversation

@seddonym
Copy link
Collaborator

@seddonym seddonym commented Feb 25, 2025

Adds module expressions - most of the work is from #188, with a few tweaks on top.

@Peter554
Copy link
Collaborator

Peter554 commented Feb 25, 2025

Consider whether the Python API for import expressions should use two-tuples or arrow-strings (i.e. "importer -> imported").

@seddonym Incase it gets buried: x-posting my remaining concern about the arrow strings here -> #188 (comment). I'm fine with whatever decision you want to take here though.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 25, 2025

CodSpeed Performance Report

Merging #192 will improve performances by 14.25%

Comparing add-module-expressions (48053df) with master (8c1bb5d)

Summary

⚡ 4 improvements
✅ 13 untouched benchmarks
🆕 2 new benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_deep_layers_large_graph_kept 19.3 ms 16.9 ms +14%
test_deep_layers_large_graph_violated 11.9 ms 10.4 ms +14.25%
test_no_chain 1.2 ms 1.1 ms +12.24%
test_no_chains 1.2 ms 1.1 ms +12.22%
🆕 test_find_matching_direct_imports N/A 37.4 ms N/A
🆕 test_find_matching_modules N/A 9.5 ms N/A

@seddonym seddonym force-pushed the add-module-expressions branch from 49a4322 to c8f6323 Compare March 6, 2025 14:21
}
}

Ok(Regex::new(&(r"^".to_owned() + &pattern_parts.join(r".") + r"$")).unwrap())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be Ok(Regex::new(&(r"^".to_owned() + &pattern_parts.join(r"\.") + r"$")).unwrap()) i.e. \. instead of .. Sorry, this one slipped past me 🙈

@seddonym seddonym force-pushed the add-module-expressions branch from 6395aa2 to 29916d0 Compare March 6, 2025 16:32
@seddonym seddonym requested a review from Peter554 March 6, 2025 16:32
Copy link
Collaborator

@Peter554 Peter554 left a comment

Choose a reason for hiding this comment

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

Thanks for finishing this off, and for finding that bug!

@seddonym seddonym force-pushed the add-module-expressions branch from 29916d0 to 48053df Compare March 6, 2025 16:52
@seddonym seddonym merged commit 601a8c5 into master Mar 6, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants