no disk caching #20
no disk caching #20
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20 +/- ##
==========================================
- Coverage 95.20% 95.18% -0.02%
==========================================
Files 1 1
Lines 396 436 +40
==========================================
+ Hits 377 415 +38
- Misses 19 21 +2
Continue to review full report at Codecov.
|
|
@jmuhlich thoughts? I removed all disk caching, moved the 2016-06 schema into source and use it in the special case that the url matches... and use lrucache for within session caching. |
src/ome_types/schema.py
Outdated
| For the special case of retrieving the 2016-06 OME Schema, use local file. | ||
| """ | ||
| if url == "http://www.openmicroscopy.org/Schemas/OME/2016-06/ome.xsd": | ||
| url = os.path.join(os.path.dirname(__file__), "ome-2016-06.xsd") |
There was a problem hiding this comment.
Cleaner to use pkg_resources.resource_stream here rather than direct file access:
https://setuptools.readthedocs.io/en/latest/pkg_resources.html#basic-resource-access
That will work from within wheels and such where the source is not expanded onto an actual filesystem. Should be able to use url = pkg_resources.resource_stream(__name__, "ome-2016-06.xsd").
There was a problem hiding this comment.
don't love the implicit setuptools dependency, though I know it's ubiquitous. but the point/goal is well-taken. will look at it
testing/test_autogen.py
Outdated
| @@ -18,12 +17,12 @@ def model(tmp_path_factory, request): | |||
| raise RuntimeError( | |||
| f"Please generate local {SRC_MODEL} before using --nogen" | |||
| ) | |||
| sys.path.insert(0, str(SRC_MODEL.parent)) | |||
| sys.path.insert(0, str(SRC_MODEL)) | |||
There was a problem hiding this comment.
Global side-effects in a test are bad news, since they can affect other tests and the results can end up depending on the other in which the tests are run. I don't have a better suggestion in this case that totally avoids modifying the path, but maybe undoing the path change after the test runs would at least contain the side-effect to the duration of the test. Also goes for the other path manipulation below.
There was a problem hiding this comment.
we can use pytest monkeypatch fixture as contextmanager... (but this is also a bit of an unusual scenario, since the model is the only thing we really test at this point... so it kinda needs to be there for all tests)
|
Where does this stand? Do you want to change anything or should I look it over one last time? |
|
lemme have one more look. I think I was convinced about something (:smile:) previously and needed to go back and implement it. |
|
ok, have another look. I didn't quite hardcode it, but I made sure that no network requests are happening (fully turned off internet and tests still pass...) and prevented the potential KeyError for |
|
And whoa do those tests fly now! |
both changes made, thanks! merging |
closes #19 (good enough for now). by adding
xmlschema.__version__andome_types.__version__to cache key, and usingshelveinstead ofpickledirectly. That has the potential to make the cache big for local development, so addedclear_schema_cachefunction that gets called on import when the cache is >30MB.