Skip to content

Conversation

@LilyFirefly
Copy link
Contributor

@LilyFirefly LilyFirefly commented Sep 5, 2025

Also refactor GraphWrapper and move scan_for_imports into the import_scanning module.

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 5, 2025

CodSpeed Instrumentation Performance Report

Merging #249 will degrade performances by 15.63%

Comparing LilyAcorn:declarative-module (e3765cb) with main (ae655cb)

Summary

❌ 4 (👁 4) regressions
✅ 19 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
👁 test_deep_layers_large_graph_kept 17.3 ms 20.5 ms -15.63%
👁 test_deep_layers_large_graph_violated 10.9 ms 12.3 ms -11.95%
👁 test_no_chain 1.1 ms 1.2 ms -11.91%
👁 test_no_chains 1.1 ms 1.2 ms -11.91%

Copy link
Collaborator

@seddonym seddonym left a comment

Choose a reason for hiding this comment

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

Great changes, thanks so much.

m.add("ParseError", py.get_type::<ParseError>())?;
m.add("CorruptCache", py.get_type::<CorruptCache>())?;
Ok(())
mod _rustgrimp {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Been meaning to do this for a while, thanks!

.ok_or(GrimpError::ModuleNotPresent(name.to_owned()))
}

fn parse_containers(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't realise there were two impl blocks! Thanks.

///
/// Returns dict[Module, set[DirectImport]].
#[pyfunction]
pub fn scan_for_imports<'py>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call. Originally the design was to have lib.rs as the Python layer but as you will have noticed, we are no longer sticking to that as the crate has grown.

@seddonym seddonym merged commit 8c9f104 into python-grimp:main Sep 8, 2025
18 checks passed
@LilyFirefly LilyFirefly deleted the declarative-module branch September 9, 2025 09:29
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