Conversation
Co-authored-by: mpu-creare <mpu-creare@users.noreply.github.com>
…portance to nodes doc
Co-authored-by: mpu-creare <mpu-creare@users.noreply.github.com>
…to explain utils._get_label
…feature/general_bug_fixes
General Bug Fixes
… version of zarr to setup. added missing webob dependency to setup
… tracking (was for debugging)
…et_data to prevent inadvertent writing attempts. changed exception catching in utils _get_from_url.
…ource to use slice coordinate_index_type to avoid incompatibility with numpy type.
…cache clearing function. Removed DiskCacheMixin. Removed lazy loading of pydap because of xarray pydap.__spec__ interaction. Fixed missing source_coordinates usage in interpolation_manager._fix_coordinates_for_none_interp.
|
@mpu-creare As far as I can tell, although the change log was updated, the 4.0.0 changes (hash_cache, zarr_cache, etc) never made their way to main before now. So, should this still be the 4.0.0 tagged release because it is the first of this new form or, because this effort did follow a different effort that considered itself 4.0.0, should this be a 4.1.0 release? |
|
Since we don't have a 4.0.0 tag at all, this should be the first 4.0.0 release. |
mpu-creare
left a comment
mpu-creare
left a comment
There was a problem hiding this comment.
Again, I have a few minor comments. Mostly to clean things up. I think it's probably worth fixing the unit test that changed.
doc/source/api.rst
Outdated
| podpac.utils.clear_cache | ||
| podpac.utils.cached_property | ||
| podpac.utils.NoCacheMixin | ||
| podpac.utils.DiskCacheMixin |
There was a problem hiding this comment.
Need to remove this from the api docs.
There was a problem hiding this comment.
I don't see this line in the release branch.
There was a problem hiding this comment.
As far as I can tell, podpac.utils.DiskCacheMixin remains in this file?
There was a problem hiding this comment.
Ah, I thought you were pointing out the NoCacheMix (I saw the highlighting in the diff and mistook it for a github highlight thing). Removed
podpac/core/cache/utils.py
Outdated
| try: | ||
| expires = np.datetime64(expires).item() | ||
| except: | ||
| except Exception: |
There was a problem hiding this comment.
I'd love to catch a more specific exception... I also would have loved a comment about why it's ok to pass
There was a problem hiding this comment.
From some brief testing, it seems failures here are always ValueErrors (I couldn't fashion a string that failed in any other way).
The catches just passing the value through is handled by the last isinstance check, which throws an error is a string went un-converted. I think a try-pass was adopted to handle the multiple different string parsing methods (because there are 2 that both fail via error, the try-pass avoids annoying case logic).
There was a problem hiding this comment.
Please add a comment so we don't have to figure this out again in the future.
There was a problem hiding this comment.
Is the one on line 46 sufficient or does it need more background?
| class TestHeterogenousInterpolation(object): | ||
| class TestHeterogenousInterpolation( | ||
| object | ||
| ): # I don't think heterogeneou interpolation works with current .interpolate() scheme. |
There was a problem hiding this comment.
replying to the commented code -- the fact that the unit test works implies that heterogenous still works?
There was a problem hiding this comment.
It seems so.
| nodes being automatically interpolated on `eval`. Now users have to create `Interpolation` nodes explicitly. To make the transition smoother, we added a convenience function `Node.interpolate` that will add an `Interpolation` node to the pipeline, restoring the original functionality. | ||
|
|
||
| ### Features | ||
| * Added the `ZarrCache`. This node enables partial caching of nodes based on the source's coordinate. This means you can |
There was a problem hiding this comment.
Did we miss anything here? Should we mention the code refactor to reduce cognitive complexity? Anything else that's new? Bugfixes, etc.
There was a problem hiding this comment.
I added the 1 new bugfix and 1 new breaking change of which I'm aware.
Most of the changes in the PR come from a previous effort that didn't make its way to a full formal/tagged release.
|
7 participants
Merging fixes from release branch into main for formal tagged release