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

Registry: pass along objects from one understand method to the next #15

Closed
phyy-nx opened this issue Dec 8, 2017 · 10 comments
Closed

Comments

@phyy-nx
Copy link
Contributor

phyy-nx commented Dec 8, 2017

Currently if one understand method from a parent Format class wishes to pass information to children Format classes, there is no way to do this. For example, if FormatCBF's understand method reads the header and returns True, then each of the children classes (mini, full, etc) re-opens the file and re-reads the header to see if it can understand it. Instead, if the understand method returned a tuple:

True, anyobject

or

False, None

Then the registry could pass on anyobject to the children. Perhaps anyobject is a header dictionary from FormatSMV, or a detector address interpreted by FormatXTC. It's implementation specific.

This would reduce file reads and allow communication of derived information to children classes.

It could be done with a minimum amount of intrusion by updating registry.py to look at what the understand method returned. If it's a bool, just call the children's understand methods. If it's a tuple, pass along the second object too.

Thoughts?

@graeme-winter
Copy link
Collaborator

Alt: return None or object

@graeme-winter
Copy link
Collaborator

=> not really an API change... I think this is a good idea and would probably help (though we do have caching in there so it is not actually going back to spinning rust to check next time... I believe)

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Dec 8, 2017 via email

@graeme-winter
Copy link
Collaborator

Anything derived from Format?

  _cache_controller = dxtbx.filecache_controller.simple_controller()
  @classmethod
  def get_cache_controller(cls):
    return cls._cache_controller

header of referenced class contains

# A shared caching layer for file-like objects.
# pseudo_file objects can be used as drop-in replacements for actual file
# handles to provide a transparent caching layer to avoid reading multiple
# times from disk or network.
#
# To create a pseudo_file instance, encapsulate a 'real' file handler
# inside a lazy cache object:
#   from dxtbx.filecache import lazy_file_cache
#   cache = lazy_file_cache(open(filename, 'rb'))
#
# Finally use a reference to the cache object to create one or many pseudo_file
# instances:
#   fh1 = cache.open()
#   from dxtbx.filecache import pseudo_file
#   fh2 = pseudo_file(cache) # equivalent
#   fh3 = pseudo_file(cache)
#   ...
#
# Each pseudo_file instance can then be treated as a proper read-only
# file handle, but will benefit from a shared cache:
#   with cache.open() as fh:
#     fh.read(100)
#     fh.readline()
#     fh.seek(500)
#     fh.read()
#     fh.readlines()
#     fh.close()
#
# To flush the cache and free the memory you can use
#     cache.close()
# This will drop the cache when all associated file handles are closed.
# To instantly drop the cache you can use
#     cache.force_close()
# Any further access attempts will then result in an exception.

@graeme-winter
Copy link
Collaborator

It is true however that your mechanism could avoid re-parsing text in headers which would be a good thing

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Dec 8, 2017 via email

@graeme-winter
Copy link
Collaborator

@Anthchirp can probably chip in here but I was under the impression that everything is using it - at least for regular formats (CBF, rayonix etc) - however it is buried under a load of metaclass witchcraft...

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Dec 8, 2017 via email

@graeme-winter
Copy link
Collaborator

but return object or None could be a useful motif to keep anyways...

@Anthchirp Anthchirp transferred this issue from cctbx/cctbx_project Apr 18, 2019
@ndevenish ndevenish changed the title dxtbx registry: pass along objects from one understand method to the next Registry: pass along objects from one understand method to the next Apr 18, 2019
@dagewa
Copy link
Member

dagewa commented Sep 30, 2024

While the idea makes sense, it would add some complexity to the code and it seems that the main motivation to avoid repeat file reads has been addressed by caching. I'm going to take the liberty to close this (though of course, please re-open if it still seems relevant).

@dagewa dagewa closed this as not planned Won't fix, can't repro, duplicate, stale Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants