Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memoizer: Persist caching to sub readers #3618

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

dgault
Copy link
Member

@dgault dgault commented Oct 7, 2020

Initial PR for testing and discussion of a possible option for passing Memoization to sub readers. In this case each reader must wrap its sub readers, which leaves a significant amount of testing required to cover the large number of readers involved.

  • MetadataOptions used to store Memoizer otpions
  • A wrap method in Memoizer to handle the wrapping
  • Each reader then wraps its sub readers

Likely to be further commits to bring this more inline with the IDR branch
New MetaxpressTiffReader also needs to be updated

@dgault dgault added the breaking label Oct 8, 2020
@joshmoore
Copy link
Member

First off, wow. I find this very exciting. I only ever approached propagating to the most necessary readers in the IDR context.

I do wonder if we're doing this across the board now if there are any other wrapping concerns we should consider. I imagine that might work by letting the Options object decide which wrappings to apply.

@sbesson sbesson added this to the 6.12.0 milestone Oct 3, 2022
@dgault dgault modified the milestones: 6.12.0, 6.13.0 Feb 6, 2023
@dgault dgault modified the milestones: 6.13.0, 6.14.0 Apr 12, 2023
@melissalinkert melissalinkert marked this pull request as draft June 12, 2023 13:19
@melissalinkert melissalinkert removed this from the 6.14.0 milestone Jun 12, 2023
@melissalinkert melissalinkert added this to the 8.0.0 milestone Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants