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.

Sign up for GitHub

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

Expose progress bar class control #1110

Merged
merged 15 commits into from
May 20, 2024
Merged

Expose progress bar class control #1110

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0db5d1c
expose progress bar class control
CodyCBakerPhD May 6, 2024
87c00e0
Merge branch 'dev' into progress_bar_class_control
CodyCBakerPhD May 15, 2024
753522e
update types
CodyCBakerPhD May 15, 2024
c9582cb
grab progress bar class from kwargs
CodyCBakerPhD May 15, 2024
faa5f52
fix
CodyCBakerPhD May 15, 2024
a333784
swap back to callable but from typing
CodyCBakerPhD May 15, 2024
dd3b3b8
swap from typing to collections
CodyCBakerPhD May 15, 2024
4ec2104
Merge branch 'dev' into progress_bar_class_control
CodyCBakerPhD May 17, 2024
2de13f9
Update CHANGELOG.md
CodyCBakerPhD May 17, 2024
d3d77a8
add test
CodyCBakerPhD May 17, 2024
4a3bba6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 17, 2024
83cc646
fix
CodyCBakerPhD May 17, 2024
bf71b91
ruff
CodyCBakerPhD May 17, 2024
65a300e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 17, 2024
0753f8a
Merge branch 'dev' into progress_bar_class_control
rly May 20, 2024
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: 9 additions & 1 deletion src/hdmf/data_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,12 @@ class GenericDataChunkIterator(AbstractDataChunkIterator):
doc="Display a progress bar with iteration rate and estimated completion time.",
default=False,
),
dict(
name="progress_bar_class",
type=callable,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea if docval is OK with this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah it's giving an error.

[2024-05-13 14:06:09,926] ERROR in app: Exception on /startup/preload-imports [GET]
Traceback (most recent call last):
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/utils.py", line 605, in dec
    a['type'] = __resolve_type(a['type'])
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/utils.py", line 527, in __resolve_type
    raise ValueError(msg)
ValueError: argtype must be a type, a str, a list, a tuple, or None - got <class 'builtin_function_or_method'>

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/garrettflynn/Documents/GitHub/nwb-guide/pyflask/apis/startup.py", line 40, in get
    import neuroconv
  File "/Users/garrettflynn/Documents/GitHub/neuroconv/src/neuroconv/__init__.py", line 1, in <module>
    from .basedatainterface import BaseDataInterface
  File "/Users/garrettflynn/Documents/GitHub/neuroconv/src/neuroconv/basedatainterface.py", line 9, in <module>
    from pynwb import NWBFile
  File "/Users/garrettflynn/miniconda3/envs/nwb-guide/lib/python3.9/site-packages/pynwb/__init__.py", line 9, in <module>
    from hdmf.spec import NamespaceCatalog
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/__init__.py", line 2, in <module>
    from .backends.hdf5.h5_utils import H5Dataset, H5RegionSlicer
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/backends/__init__.py", line 1, in <module>
    from . import hdf5
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/backends/hdf5/__init__.py", line 1, in <module>
    from . import h5_utils, h5tools
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/backends/hdf5/h5_utils.py", line 20, in <module>
    from ...data_utils import DataIO, AbstractDataChunkIterator
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/data_utils.py", line 140, in <module>
    class GenericDataChunkIterator(AbstractDataChunkIterator):
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/data_utils.py", line 197, in GenericDataChunkIterator
    def __init__(self, **kwargs):
  File "/Users/garrettflynn/Documents/GitHub/hdmf/src/hdmf/utils.py", line 608, in dec
    raise Exception(msg)
Exception: docval for progress_bar_class: error parsing argument type: argtype must be a type, a str, a list, a tuple, or None - got <class 'builtin_function_or_method'>

Is there a simple way to fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rly @oruebel I want to specify a callable in a function/class wrapped by docval

Is there any way to do that or otherwise circumvent it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that callable is not a type in Python but rather a built-in function. If you want to restrict to progress bar classes from tqdm then you could set the type to tqdm.std, which I believe is the common base class for progress bars in tqdm. Or you could make a list of all the progress bar types you want to allow. I believe docval allows this to be specified as a string tqdm.std so that you should not need to import tqdm in order to specify this for docval. Alternatively, if you want any class to pass validation, then I believe you could set type to None.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tqdm.std is the module, not a parent class; tqdm.tqdm is the thing they recommend using as a base class when extending, but in general you could use any generic iterable wrapper if you wanted to

I'll just use the override for now

doc="The progress bar class to use. Defaults to tqdm.tqdm if the TQDM package is installed.",
default=None,
),
dict(
name="progress_bar_options",
type=None,
Expand Down Expand Up @@ -277,11 +283,13 @@ def __init__(self, **kwargs):
try:
from tqdm import tqdm

progress_bar_class = progress_bar_class or tqdm

if "total" in self.progress_bar_options:
warn("Option 'total' in 'progress_bar_options' is not allowed to be over-written! Ignoring.")
self.progress_bar_options.pop("total")

self.progress_bar = tqdm(total=self.num_buffers, **self.progress_bar_options)
self.progress_bar = progress_bar_class(total=self.num_buffers, **self.progress_bar_options)
except ImportError:
warn(
"You must install tqdm to use the progress bar feature (pip install tqdm)! "
Expand Down
Loading