Move all file-load code into iris.io.loading. #6199
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6199 +/- ##
=======================================
Coverage ? 89.88%
=======================================
Files ? 89
Lines ? 23319
Branches ? 4338
=======================================
Hits ? 20960
Misses ? 1628
Partials ? 731 ☔ View full report in Codecov by Sentry. |
| LOAD_POLICY = LoadPolicy() | ||
|
|
||
|
|
||
| def _combine_cubes(cubes, options, merge_require_unique): |
There was a problem hiding this comment.
Is this ultimately where we want the public function _combine_cubes to live? I could see an argument for some of this code belonging in a separate file like _merge.py or _concatenate.py.
There was a problem hiding this comment.
I guess for separation of concerns we could do that. So we could separate out the 'combine' operation from iris.io.loading to create an iris.combine, if that seems preferable.
But as it's private for now, that doesn't really affect this PR.
Note that theiris._merge and iris._concatenate modules are private, so the public access to these is only through the CubeList methods.
The follow-on PR currently proposes to add CubeList.combine[_cubes] methods, but it does also publish the main function as iris.io.loading.combine_cubes.
We can change that, but I'd prefer to address it separately.
I don't think this would affect the new iris.io.loading module, : that would anyway still have a place, since it contains the main load_xxx functions (and supporting routines).
| # | ||
| # This file is part of Iris and is released under the BSD license. | ||
| # See LICENSE in the root of the repository for full licensing details. | ||
| """Loading mechanism and functions.""" |
There was a problem hiding this comment.
Does this naming convention of iris.io.loading make sense with respect to the existing structure of iris.io? I notice that iris.io.__init__ contains several load functions (e.g. load_files). Would it make sense to move this code also into iris.io.loading or is there some module name which would better explain the separation of these functions?
| @@ -36,7 +36,7 @@ def test_callable_function(self): | |||
| import iris | |||
|
|
|||
| result = _qualname(iris.load) | |||
| self.assertEqual(result, "iris.load") | |||
| self.assertEqual(result, "iris.io.loading.load") | |||
There was a problem hiding this comment.
I believe this has now been changed so that self.assertEqual(x, y) is replaced by assert x == y.
| import os.path | ||
| import threading | ||
| from typing import Callable, Literal, Mapping | ||
| from typing import Callable, Literal | ||
|
|
||
| import iris._constraints | ||
| import iris.config | ||
| import iris.io |
There was a problem hiding this comment.
I notice now that the only place iris.io is being called is the line save = iris.io.save. Would it not make more sense to replace this with from iris.io import save?
Refactor to co-locate all the iris loading into a separate module, for simpler organisation + easier maintenance,
(including + mostly triggered by recent additions as per #6168)