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

Added get_config() function to retrieve information #187

Merged
merged 5 commits into from
Oct 20, 2022

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Oct 18, 2022

This PR:

  • adds a debug log when successfully registering a filter.
  • adds and document a get_config() function with provides access to both build-time options and runtime registered filters.

Previously hdf5plugin.config was available (and still is) to provide build time config but it is not documented.

I hesitated between providing a dedicated get_registered_filters() function and providing a more general get_config() one as this PR proposes. I chose the later to avoid having too many functions for accessing debug/more advanced information. Any other opinion? Better naming?

closes #185

@t20100 t20100 added this to the Next release milestone Oct 18, 2022
@t20100 t20100 requested review from kif and vasole October 18, 2022 15:37
@vasole
Copy link
Member

vasole commented Oct 19, 2022

Why is the information about the constants missing?

Well, I find it somewhere else. Probably it is somehow fetched from __init__.py instead of being explicitly written in the .rst file

@vasole
Copy link
Member

vasole commented Oct 19, 2022

The situation is:

  • we need to know how hdf5plugin was configured
  • we need to know what filters have been registered
  • we need to know what registered filters come form hdf5plugin and which ones do not

@t20100
Copy link
Member Author

t20100 commented Oct 19, 2022

get_config() provides:

  • how hdf5plugin was configured
  • what registered filters come form hdf5plugin

To me the last point "what filters have been registered" is in a wider scope than hdf5plugin and would be better suited in h5py

@t20100
Copy link
Member Author

t20100 commented Oct 19, 2022

Yes, the constants are now documented with autodoc to avoid duplication between the docstring and the rst.

@vasole
Copy link
Member

vasole commented Oct 19, 2022

I agree it is a more generic functionality to retrieve all filters.

However, in the case we have skipped a filter because it was already loaded, I think we should somehow let the user retrieve that information without having to toggle the logging level and (perhaps) restart again.

@aragilar suggested to provide hdf5plugin loaded filters and used path (obviously if we load them we use hdf5plugin.PLUGIN_PATH) but if we do not load it because already there then the path is unknown...

@vasole
Copy link
Member

vasole commented Oct 19, 2022

Basically I am suggesting to add yield name, ("unknown", "unknown") after _logger.info("%s filter already loaded, skip it.", name)

@t20100
Copy link
Member Author

t20100 commented Oct 20, 2022

Yes I agree.

As it is, one can already find this information by checking both embedded_filters and registered_filters, but all this information can be included in registered_filters.

@t20100
Copy link
Member Author

t20100 commented Oct 20, 2022

I added a status message to the registered_filters:
To provide something like this:

In [1]: import hdf5plugin

In [2]: hdf5plugin.get_config().registered_filters
Out[2]: 
{'blosc': FilterStatus(status='Missing: not embedded in hdf5plugin', filename=None),
 'bshuf': FilterStatus(status='Missing: not embedded in hdf5plugin', filename=None),
 'bzip2': FilterStatus(status='Missing: not embedded in hdf5plugin', filename=None),
 'lz4': FilterStatus(status='Registered', filename='/hdf5plugin/plugins/libh5lz4.dylib'),
 'zfp': FilterStatus(status='Missing: not embedded in hdf5plugin', filename=None),
 'zstd': FilterStatus(status='Missing: not embedded in hdf5plugin', filename=None),
 'fcidecomp': FilterStatus(status='Missing: not embedded in hdf5plugin', filename=None)}

but it's maybe too much, this is already in the logging message, maybe better to keep it simple.

@vasole
Copy link
Member

vasole commented Oct 20, 2022

I think the requests are already fulfilled. The rest is just a question of taste.

@vasole vasole merged commit 0be2e16 into silx-kit:main Oct 20, 2022
@t20100 t20100 deleted the add-get_config branch October 26, 2022 07:26
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

Successfully merging this pull request may close these issues.

Improve logging/information about hdf5plugin loaded filters
2 participants