Skip to content
This repository has been archived by the owner on Jun 21, 2022. It is now read-only.

Actually use cache for newly-constructed VirtualArrays #231

Merged
merged 1 commit into from
Jan 30, 2020

Conversation

nsmith-
Copy link
Contributor

@nsmith- nsmith- commented Jan 30, 2020

Also require cache to conform to MutableMapping interface, i.e. KeyError
is the only exception that indicates trouble.

Also require cache to conform to MutableMapping interface, i.e. KeyError
is the only exception that indicates trouble.
@jpivarski
Copy link
Member

I had a question about one line, but this looks right. Give me a checkmark or thumbs up or something and I'll merge it.

@nsmith-
Copy link
Contributor Author

nsmith- commented Jan 30, 2020

My one other complaint is there's a lot of key in cache checks that may be optional if we trust the virtual array to have correct type before materialization. This isn't a huge deal, but there may be unnecessary overhead in the cases where the cache is backed by something slow.

@jpivarski
Copy link
Member

You're right. In fact, if we trust try ... catch KeyError logic, then the cache doesn't have to have a __contains__ operation defined, and it can be more thread-safe because of the time that elapses between the key check and the extraction.

@jpivarski jpivarski merged commit d972336 into scikit-hep:master Jan 30, 2020
@nsmith-
Copy link
Contributor Author

nsmith- commented Jan 30, 2020

In python's Mapping abc, __contains__ is not abstract because of this very logic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants