Skip to content

Commit 7782fb8

Browse files
committed
Support is_lazy when building graph
1 parent e6e5cc2 commit 7782fb8

23 files changed

Lines changed: 360 additions & 84 deletions

‎CHANGELOG.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ Changelog
55
latest
66
------
77

8+
* Support explicit lazy imports.
89
* Raise ``ValueError`` instead of panicking when ``Graph.add_import`` is called with only one of
910
``line_number`` and ``line_contents``.
1011

‎docs/usage.rst‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ Methods for analysing direct imports
164164

165165
This method should not be used to determine whether an import is present:
166166
some of the imports in the graph may have no available metadata. For example, if an import
167-
has been added by the ``add_import`` method without the ``line_number`` and ``line_contents`` specified, then
167+
has been added by the ``add_import`` method without the optional arguments, then
168168
calling this method on the import will return an empty list. If you want to know whether the import is present,
169169
use ``direct_import_exists``.
170170

@@ -174,6 +174,7 @@ Methods for analysing direct imports
174174
{
175175
'importer': 'mypackage.importer',
176176
'imported': 'mypackage.imported',
177+
'is_lazy': False,
177178
'line_number': 5,
178179
'line_contents': 'from mypackage import imported',
179180
},
@@ -185,6 +186,7 @@ Methods for analysing direct imports
185186
:param str importer: A module name.
186187
:param str imported: A module name.
187188
:return: A list of any available metadata for imports between two modules.
189+
The ``is_lazy`` item indicates whether the import is an `explicit lazy import`_.
188190
:rtype: List of dictionaries with the structure shown above. If you want to use type annotations, you may use the
189191
``grimp.DetailedImport`` TypedDict for each dictionary.
190192

@@ -560,18 +562,23 @@ Methods for manipulating the graph
560562
:param str module: The name of a module, for example ``'mypackage.foo'``.
561563
:return: None
562564

563-
.. py:function:: ImportGraph.add_import(importer, imported, line_number=None, line_contents=None)
565+
.. py:function:: ImportGraph.add_import(importer, imported, is_lazy=False, line_number=None, line_contents=None)
564566
565567
Add a direct import between two modules to the graph. If the modules are not already
566568
present, they will be added to the graph.
567569

570+
The optional arguments containing the import details are intended to be called all together, or not at all,
571+
though ``is_lazy`` is optional for backward compatibility. This data is available later via ``get_import_details``.
572+
568573
:param str importer: The name of the module that is importing the other module.
569574
:param str imported: The name of the module being imported.
575+
:param bool is_lazy: Whether the import is an explicit lazy import.
570576
:param int line_number: The line number of the import statement in the module.
571577
:param str line_contents: The line that contains the import statement.
572578
:return: None
573579

574-
:raises: ``ValueError`` if only one of ``line_number`` or ``line_contents`` is supplied.
580+
:raises: ``ValueError`` if only one of ``line_number`` or ``line_contents`` is supplied,
581+
or if ``is_lazy`` is supplied without ``line_number`` and ``line_contents``.
575582

576583
.. py:function:: ImportGraph.remove_import(importer, imported)
577584
@@ -621,3 +628,4 @@ Module expressions
621628

622629
.. _namespace packages: https://docs.python.org/3/glossary.html#term-namespace-package
623630
.. _namespace portion: https://docs.python.org/3/glossary.html#term-portion
631+
.. _explicit lazy import: https://docs.python.org/3.15/reference/simple_stmts.html#lazy

‎rust/src/caching.rs‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,27 @@ impl<'a, 'py> FromPyObject<'a, 'py> for ImportsByModule {
7575
}
7676
}
7777

78+
/// The version of the data cache file format.
79+
///
80+
/// Bump this whenever the serialized structure changes. On read, a file with a
81+
/// different (or missing) version is treated as a version mismatch and rebuilt,
82+
/// rather than being mistaken for a corrupt file.
83+
const CACHE_SCHEMA_VERSION: u32 = 2;
84+
7885
fn serialize_imports_by_module(
7986
imports_by_module: &HashMap<Module, HashSet<DirectImport>>,
8087
) -> String {
81-
let raw_map: HashMap<&str, Vec<(&str, usize, &str)>> = imports_by_module
88+
// Fields are ordered to match the `get_import_details` dict (`imported`, `is_lazy`,
89+
// `line_number`, `line_contents`); `importer` is the map key.
90+
let raw_map: HashMap<&str, Vec<(&str, bool, usize, &str)>> = imports_by_module
8291
.iter()
8392
.map(|(module, imports)| {
84-
let imports_vec: Vec<(&str, usize, &str)> = imports
93+
let imports_vec: Vec<(&str, bool, usize, &str)> = imports
8594
.iter()
8695
.map(|import| {
8796
(
8897
import.imported.as_str(),
98+
import.is_lazy,
8999
import.line_number,
90100
import.line_contents.as_str(),
91101
)
@@ -95,16 +105,33 @@ fn serialize_imports_by_module(
95105
})
96106
.collect();
97107

98-
serde_json::to_string(&raw_map).expect("Failed to serialize to JSON")
108+
let envelope = serde_json::json!({
109+
"version": CACHE_SCHEMA_VERSION,
110+
"imports_by_module": raw_map,
111+
});
112+
113+
serde_json::to_string(&envelope).expect("Failed to serialize to JSON")
99114
}
100115

101116
pub fn parse_json_to_map(
102117
json_str: &str,
103118
filename: &str,
104119
) -> GrimpResult<HashMap<Module, HashSet<DirectImport>>> {
105-
let raw_map: HashMap<String, Vec<(String, usize, String)>> = serde_json::from_str(json_str)
120+
// Parse into a generic value first, so we can distinguish genuinely corrupt
121+
// JSON from a cache file written by a different format version (e.g. by an
122+
// older Grimp). The latter should be silently rebuilt, not warned about.
123+
let value: serde_json::Value = serde_json::from_str(json_str)
106124
.map_err(|_| GrimpError::CorruptCache(filename.to_string()))?;
107125

126+
let version = value.get("version").and_then(|v| v.as_u64());
127+
if version != Some(CACHE_SCHEMA_VERSION as u64) {
128+
return Err(GrimpError::CacheVersionMismatch(filename.to_string()));
129+
}
130+
131+
let raw_map: HashMap<String, Vec<(String, bool, usize, String)>> =
132+
serde_json::from_value(value.get("imports_by_module").cloned().unwrap_or_default())
133+
.map_err(|_| GrimpError::CorruptCache(filename.to_string()))?;
134+
108135
let mut parsed_map: HashMap<Module, HashSet<DirectImport>> = HashMap::new();
109136

110137
for (module_name, imports) in raw_map {
@@ -113,13 +140,15 @@ pub fn parse_json_to_map(
113140
};
114141
let import_set: HashSet<DirectImport> = imports
115142
.into_iter()
116-
.map(|(imported, line_number, line_contents)| DirectImport {
117-
importer: module_name.clone(),
118-
imported,
119-
line_number,
120-
line_contents,
121-
is_lazy: false, // TODO get working with cache.
122-
})
143+
.map(
144+
|(imported, is_lazy, line_number, line_contents)| DirectImport {
145+
importer: module_name.clone(),
146+
imported,
147+
line_number,
148+
line_contents,
149+
is_lazy,
150+
},
151+
)
123152
.collect();
124153
parsed_map.insert(module, import_set);
125154
}

‎rust/src/errors.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ pub enum GrimpError {
3636

3737
#[error("Could not use corrupt cache file {0}.")]
3838
CorruptCache(String),
39+
40+
#[error("Cache file {0} was written by a different version of Grimp.")]
41+
CacheVersionMismatch(String),
3942
}
4043

4144
pub type GrimpResult<T> = Result<T, GrimpError>;
@@ -55,6 +58,9 @@ impl From<GrimpError> for PyErr {
5558
line_number, text, ..
5659
} => PyErr::new::<exceptions::ParseError, _>((line_number, text)),
5760
GrimpError::CorruptCache(_) => exceptions::CorruptCache::new_err(value.to_string()),
61+
GrimpError::CacheVersionMismatch(_) => {
62+
exceptions::CacheVersionMismatch::new_err(value.to_string())
63+
}
5864
}
5965
}
6066
}

‎rust/src/exceptions.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ create_exception!(_rustgrimp, ModuleNotPresent, PyException);
66
create_exception!(_rustgrimp, NoSuchContainer, PyException);
77
create_exception!(_rustgrimp, InvalidModuleExpression, PyException);
88
create_exception!(_rustgrimp, CorruptCache, PyException);
9+
create_exception!(_rustgrimp, CacheVersionMismatch, PyException);
910

1011
// We need to use here `pyclass(extends=PyException)` instead of `create_exception!`
1112
// since the exception contains custom data. See:

‎rust/src/graph/graph_manipulation.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ impl Graph {
120120
imported: ModuleToken,
121121
line_number: u32,
122122
line_contents: &str,
123+
is_lazy: bool,
123124
) {
124125
self.imports
125126
.entry(importer)
@@ -137,7 +138,7 @@ impl Graph {
137138
self.import_details
138139
.entry((importer, imported))
139140
.or_default()
140-
.insert(PyImportDetails::new(line_number, line_contents));
141+
.insert(PyImportDetails::new(line_number, line_contents, is_lazy));
141142
}
142143
}
143144

‎rust/src/graph/mod.rs‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -264,27 +264,39 @@ impl GraphWrapper {
264264
Ok(self.get_visible_module_by_name(module)?.is_squashed())
265265
}
266266

267-
#[pyo3(signature = (*, importer, imported, line_number=None, line_contents=None))]
267+
#[pyo3(signature = (*, importer, imported, is_lazy=None, line_number=None, line_contents=None))]
268268
pub fn add_import(
269269
&mut self,
270270
importer: &str,
271271
imported: &str,
272+
is_lazy: Option<bool>,
272273
line_number: Option<u32>,
273274
line_contents: Option<&str>,
274275
) -> PyResult<()> {
275-
match (line_number, line_contents) {
276-
(Some(line_number), Some(line_contents)) => {
276+
match (line_number, line_contents, is_lazy) {
277+
(None, None, None) => {
277278
let importer = self._graph.get_or_add_module(importer).token();
278279
let imported = self._graph.get_or_add_module(imported).token();
279-
self._graph
280-
.add_detailed_import(importer, imported, line_number, line_contents);
280+
self._graph.add_import(importer, imported);
281281
}
282-
(None, None) => {
282+
(Some(line_number), Some(line_contents), is_lazy) => {
283+
let is_lazy = is_lazy.unwrap_or(false);
283284
let importer = self._graph.get_or_add_module(importer).token();
284285
let imported = self._graph.get_or_add_module(imported).token();
285-
self._graph.add_import(importer, imported);
286+
self._graph.add_detailed_import(
287+
importer,
288+
imported,
289+
line_number,
290+
line_contents,
291+
is_lazy,
292+
);
286293
}
287-
_ => {
294+
(_, _, Some(_is_lazy)) => {
295+
return Err(PyValueError::new_err(
296+
"You must provide line_number and line_contents when providing is_lazy.",
297+
));
298+
}
299+
(_, _, None) => {
288300
return Err(PyValueError::new_err(
289301
"Expected line_number and line_contents, or neither.",
290302
));
@@ -406,6 +418,7 @@ impl GraphWrapper {
406418
imported.name(),
407419
import_details.line_number(),
408420
import_details.line_contents(),
421+
import_details.is_lazy(),
409422
)
410423
})
411424
.sorted()
@@ -421,6 +434,7 @@ impl GraphWrapper {
421434
"line_contents",
422435
import_details.line_contents.into_py_any(py).unwrap(),
423436
),
437+
("is_lazy", import_details.is_lazy.into_py_any(py).unwrap()),
424438
]
425439
.into_py_dict(py)
426440
.unwrap()
@@ -671,6 +685,7 @@ struct ImportDetails {
671685
imported: String,
672686
line_number: u32,
673687
line_contents: String,
688+
is_lazy: bool,
674689
}
675690

676691
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, new)]
@@ -758,6 +773,9 @@ pub struct PyImportDetails {
758773

759774
#[getset(get_copy = "pub")]
760775
interned_line_contents: DefaultSymbol,
776+
777+
#[getset(get_copy = "pub")]
778+
is_lazy: bool,
761779
}
762780

763781
impl PyImportDetails {

‎rust/src/import_scanning.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,7 @@ fn to_py_direct_imports<'a>(
201201
kwargs
202202
.set_item("line_contents", &rust_import.line_contents)
203203
.unwrap();
204-
kwargs
205-
.set_item("is_lazy", &rust_import.is_lazy)
206-
.unwrap();
204+
kwargs.set_item("is_lazy", rust_import.is_lazy).unwrap();
207205
let py_direct_import = py_direct_import_class.call((), Some(&kwargs)).unwrap();
208206
pyset.add(&py_direct_import).unwrap();
209207
}

‎rust/src/lib.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ mod _rustgrimp {
2929

3030
#[pymodule_export]
3131
use crate::exceptions::{
32-
CorruptCache, InvalidModuleExpression, ModuleNotPresent, NoSuchContainer, ParseError,
32+
CacheVersionMismatch, CorruptCache, InvalidModuleExpression, ModuleNotPresent,
33+
NoSuchContainer, ParseError,
3334
};
3435
}

‎src/grimp/adaptors/caching.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ def _read_data_map_file(self) -> dict[Module, set[DirectImport]]:
203203
except rust.CorruptCache:
204204
logger.warning(f"Could not use corrupt cache file {data_cache_filename}.")
205205
return {}
206+
except rust.CacheVersionMismatch:
207+
logger.info(
208+
f"Cache file {data_cache_filename} was written by a different version of Grimp; "
209+
"rebuilding."
210+
)
211+
return {}
206212

207213
logger.info(f"Used cache data file {data_cache_filename}.")
208214
return imports_by_module

0 commit comments

Comments
 (0)