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

Improve organization #485

Merged
merged 21 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
# Upcoming

### Deprecation (API)
* Certain low-level functions have been marked as private (such as the former `nwbinspector.register_checks.auto_parse`) indicating they should not have been imported by downstream users. [#485](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/485)
* Various inappropriate imports from certain submodules have been hard deprecated (e.g., `from nwbinspector.inspector_tools import natsorted`). [#485](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/485)

### Pending Deprecation (API)
* The `inspector_tools`, `register_check`, and `` submodules have been soft deprecated and will be removed in the next major release. [#485](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/485)

### Improvements

* Update util function `is_ascending_series` to discard nan values and add `check_timestamps_without_nans` fun to check if timestamps contain NaN values [#476](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/476)
* Updated the import structure to match modern Python packaging standards. [#485](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/485)



# v0.4.37

Expand Down
2 changes: 1 addition & 1 deletion docs/api/nwbinspector.rst
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Core Library
============

.. automodule:: nwbinspector.nwbinspector
.. automodule:: nwbinspector._inspection
4 changes: 3 additions & 1 deletion docs/api/register_check.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ Data Classes and Check Registration

All common data class used across the package, as well as the decorator for adding a check function to the registry.

.. automodule:: nwbinspector.register_checks
.. automodule:: nwbinspector._types

.. automodule:: nwbinspector._registration
9 changes: 6 additions & 3 deletions docs/api/tools.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Tools for Organizating and Displaying Reports
=============================================
Tools for Organizing and Displaying Reports
===========================================

.. automodule:: nwbinspector
:members:
Expand All @@ -9,5 +9,8 @@ Tools for Organizating and Displaying Reports
.. toctree::
:maxdepth: 4

.. automodule:: nwbinspector.inspector_tools
.. automodule:: nwbinspector._organization
:noindex:

.. automodule:: nwbinspector._formatting
:noindex:
38 changes: 36 additions & 2 deletions docs/conf.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import os
import sys
from pathlib import Path

from nwbinspector import available_checks
from collections import defaultdict

sys.path.append(str(Path(__file__).parent))
from conf_extlinks import extlinks, intersphinx_mapping
from gen_checks_by_importance import gen_checks_by_importance

sys.path.insert(0, Path(__file__).resolve().parents[1])

Expand Down Expand Up @@ -73,4 +76,35 @@ def setup(app):
app.connect("autodoc-process-docstring", add_refs_to_docstrings)


gen_checks_by_importance()
def _gen_checks_by_importance():
dd = defaultdict(list)

for x in available_checks:
dd[x.importance.name.replace("_", " ")].append(x)

generate_checks_rst_file = os.path.join(os.path.dirname(os.path.abspath(__file__)), "checks_by_importance.rst")
with open(generate_checks_rst_file, "w") as f:
f.write(
"""Checks by Importance
======================

This section lists the available checks organized by their importance level.

"""
)

for importance_level, checks in dd.items():
f.write(
f"""{importance_level}
{'-' * (len(f'{importance_level}') + 1)}

"""
)

for check in checks:
f.write(f"* :py:func:`~{check.__module__}.{check.__name__}`\n")

f.write("\n")


_gen_checks_by_importance()
35 changes: 0 additions & 35 deletions docs/gen_checks_by_importance.py

This file was deleted.

4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
long_description = f.read()
with open(root / "requirements.txt") as f:
install_requires = f.readlines()
with open(root / "src" / "nwbinspector" / "version.py") as f:
with open(root / "src" / "nwbinspector" / "_version.py") as f:
version = f.read()

# Instantiate the testing configuration file from the base file `base_test_config.json`
Expand All @@ -34,7 +34,7 @@
install_requires=install_requires,
# zarr<2.18.0 because of https://github.com/NeurodataWithoutBorders/nwbinspector/pull/460
extras_require=dict(dandi=["dandi>=0.39.2", "zarr<2.18.0"], zarr=["hdmf_zarr>=0.3.0", "zarr<2.18.0"]),
entry_points={"console_scripts": ["nwbinspector=nwbinspector.nwbinspector:inspect_all_cli"]},
entry_points={"console_scripts": ["nwbinspector=nwbinspector._inspection_cli:_inspect_all_cli"]},
license="BSD-3-Clause",
classifiers=[
"Development Status :: 4 - Beta",
Expand Down
57 changes: 40 additions & 17 deletions src/nwbinspector/__init__.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,47 @@
from .version import __version__
from .register_checks import available_checks, Importance
from .nwbinspector import (
InspectorOutputJSONEncoder,
from ._version import __version__
from ._registration import available_checks, register_check
from ._types import Importance, Severity, InspectorMessage
from ._configuration import load_config, validate_config, configure_checks
from ._inspection import (
inspect_all,
inspect_nwbfile,
inspect_nwbfile_object,
run_checks,
load_config,
)
from .nwbinspector import inspect_nwb # TODO: remove after 7/1/2023
from .checks.ecephys import *
from .checks.general import *
from .checks.image_series import *
from .checks.images import *
from .checks.nwb_containers import *
from .checks.nwbfile_metadata import *
from .checks.ogen import *
from .checks.ophys import *
from .checks.tables import *
from .checks.time_series import *
from .checks.icephys import *
from ._inspection import inspect_nwb # TODO: remove after 7/1/2023
from ._formatting import (
format_messages,
print_to_console,
save_report,
MessageFormatter,
FormatterOptions,
InspectorOutputJSONEncoder,
)
from ._organization import organize_messages
from .checks import *

default_check_registry = {check.__name__: check for check in available_checks}

__all__ = [
"available_checks",
"default_check_registry",
"register_check",
"Importance",
"Severity",
"InspectorMessage",
"load_config",
"configure_checks",
"InspectorOutputJSONEncoder",
"inspect_all",
"inspect_nwbfile",
"inspect_nwbfile_object",
"inspect_nwb", # TODO: remove after 7/1/2023
"run_checks",
"format_messages",
"print_to_console",
"save_report",
"MessageFormatter",
"FormatterOptions",
"organize_messages",
"__version__",
]
137 changes: 137 additions & 0 deletions src/nwbinspector/_configuration.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
"""Primary functions for inspecting NWBFiles."""

import json
import jsonschema
from pathlib import Path
from enum import Enum
from typing import Optional, List
from types import FunctionType
from packaging.version import Version

import yaml

from . import available_checks
from ._registration import InspectorMessage, Importance
from nwbinspector.utils._utils import (
PathType,
)

INTERNAL_CONFIGS = dict(dandi=Path(__file__).parent / "internal_configs" / "dandi.inspector_config.yaml")


def validate_config(config: dict):
"""Validate an instance of configuration against the official schema."""
with open(file=Path(__file__).parent / "config.schema.json", mode="r") as fp:
schema = json.load(fp=fp)
jsonschema.validate(instance=config, schema=schema)


def _copy_function(function):
"""Copy the core parts of a given function, excluding wrappers, then return a new function."""
copied_function = FunctionType(
function.__code__, function.__globals__, function.__name__, function.__defaults__, function.__closure__
)

# in case f was given attrs (note this dict is a shallow copy)
copied_function.__dict__.update(function.__dict__)
return copied_function


def copy_check(check):
"""
Copy a check function so that internal attributes can be adjusted without changing the original function.

Required to ensure our configuration of functions in the registry does not effect the registry itself.

Also copies the wrapper for auto-parsing results,
see https://github.com/NeurodataWithoutBorders/nwbinspector/pull/218 for explanation.
"""
copied_check = _copy_function(function=check)
copied_check.__wrapped__ = _copy_function(function=check.__wrapped__)
return copied_check


def load_config(filepath_or_keyword: PathType) -> dict:
"""
Load a config dictionary either via keyword search of the internal configs, or an explicit filepath.

Currently supported keywords are:
- 'dandi'
For all DANDI archive related practices, including validation and upload.
"""
file = INTERNAL_CONFIGS.get(filepath_or_keyword, filepath_or_keyword)
with open(file=file, mode="r") as stream:
config = yaml.load(stream=stream, Loader=yaml.Loader)
return config


def configure_checks(
checks: list = available_checks,
config: Optional[dict] = None,
ignore: Optional[List[str]] = None,
select: Optional[List[str]] = None,
importance_threshold: Importance = Importance.BEST_PRACTICE_SUGGESTION,
) -> list:
"""
Filter a list of check functions (the entire base registry by default) according to the configuration.

Parameters
----------
config : dict
Dictionary valid against our JSON configuration schema.
Can specify a mapping of importance levels and list of check functions whose importance you wish to change.
Typically loaded via json.load from a valid .json file
checks : list of check functions
Defaults to all registered checks.
ignore: list, optional
Names of functions to skip.
select: list, optional
If loading all registered checks, this can be shorthand for selecting only a handful of them.
importance_threshold : string, optional
Ignores all tests with an post-configuration assigned importance below this threshold.
Importance has three levels:

CRITICAL
- potentially incorrect data
BEST_PRACTICE_VIOLATION
- very suboptimal data representation
BEST_PRACTICE_SUGGESTION
- improvable data representation

The default is the lowest level, BEST_PRACTICE_SUGGESTION.
"""
if ignore is not None and select is not None:
raise ValueError("Options 'ignore' and 'select' cannot both be used.")
if importance_threshold not in Importance:
raise ValueError(
f"Indicated importance_threshold ({importance_threshold}) is not a valid importance level! Please choose "
"from [CRITICAL_IMPORTANCE, BEST_PRACTICE_VIOLATION, BEST_PRACTICE_SUGGESTION]."
)
if config is not None:
validate_config(config=config)
checks_out = []
ignore = ignore or []
for check in checks:
mapped_check = copy_check(check=check)
for importance_name, func_names in config.items():
if check.__name__ in func_names:
if importance_name == "SKIP":
ignore.append(check.__name__)
continue
mapped_check.importance = Importance[importance_name]
# Output wrappers are apparently parsed at time of wrapping not of time of output return...
# Attempting to re-wrap the copied function if the importance level is being adjusted...
# From https://github.com/NeurodataWithoutBorders/nwbinspector/issues/302
# new_check_wrapper = _copy_function(function=mapped_check.__wrapped__)
# new_check_wrapper.importance = Importance[importance_name]
# mapped_check.__wrapped__ = new_check_wrapper
checks_out.append(mapped_check)
else:
checks_out = checks
if select:
checks_out = [x for x in checks_out if x.__name__ in select]
elif ignore:
checks_out = [x for x in checks_out if x.__name__ not in ignore]
if importance_threshold:
checks_out = [x for x in checks_out if x.importance.value >= importance_threshold.value]
return checks_out
Loading
Loading