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

Give users options to condense or eliminate progress bars #556

Merged
merged 5 commits into from
Jan 23, 2025
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
3 changes: 2 additions & 1 deletion servicex/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
from servicex.databinder_models import Sample, General, ServiceXSpec
from servicex.servicex_client import deliver
from servicex.servicex_client import deliver, ProgressBarFormat
from .models import ResultDestination
import servicex.dataset as dataset
import servicex.query as query
Expand All @@ -44,4 +44,5 @@
"deliver",
"dataset",
"query",
"ProgressBarFormat",
]
12 changes: 9 additions & 3 deletions servicex/dataset_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,16 @@ async def as_signed_urls_async(
display_progress: bool = True,
provided_progress: Optional[Progress] = None,
return_exceptions: bool = False,
overall_progress: bool = False,
) -> List[Union[TransformedResults, BaseException]]:
# preflight auth
if self.datasets:
await self.datasets[0].servicex._get_authorization()
with ExpandableProgress(display_progress, provided_progress) as progress:
with ExpandableProgress(
display_progress, provided_progress, overall_progress=overall_progress
) as progress:
self.tasks = [
d.as_signed_urls_async(provided_progress=progress)
d.as_signed_urls_async(provided_progress=progress, dataset_group=True)
for d in self.datasets
]
return await asyncio.gather(
Expand All @@ -85,11 +88,14 @@ async def as_files_async(
display_progress: bool = True,
provided_progress: Optional[Progress] = None,
return_exceptions: bool = False,
overall_progress: bool = False,
) -> List[Union[TransformedResults, BaseException]]:
# preflight auth
if self.datasets:
await self.datasets[0].servicex._get_authorization()
with ExpandableProgress(display_progress, provided_progress) as progress:
with ExpandableProgress(
display_progress, provided_progress, overall_progress=overall_progress
) as progress:
self.tasks = [
d.as_files_async(provided_progress=progress) for d in self.datasets
]
Expand Down
19 changes: 15 additions & 4 deletions servicex/expandable_progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,12 @@ def add_task(self, param, start, total):
task_id = self.progress.add_task(
param, start=start, total=total, visible=False
)
new_task = ProgressCounts(param, task_id, start=start, total=total)
new_task = ProgressCounts(
"Transform" if param.endswith("Transform") else param,
task_id,
start=start,
total=total,
)
self.progress_counts[task_id] = new_task
return task_id
if self.display_progress and not self.overall_progress:
Expand All @@ -156,13 +161,15 @@ def add_task(self, param, start, total):
def update(self, task_id, task_type, total=None, completed=None, **fields):

if self.display_progress and self.overall_progress:
if task_type.endswith("Transform"):
ponyisi marked this conversation as resolved.
Show resolved Hide resolved
task_type = "Transform"
# Calculate and update
overall_completed = 0
overall_total = 0
if completed:
self.progress_counts[task_id].completed = completed

elif total:
if total:
self.progress_counts[task_id].total = total

for task in self.progress_counts:
Expand Down Expand Up @@ -199,7 +206,7 @@ def update(self, task_id, task_type, total=None, completed=None, **fields):

def start_task(self, task_id, task_type):
if self.display_progress and self.overall_progress:
if task_type == "Transform":
if task_type.endswith("Transform"):
self.progress.start_task(task_id=self.overall_progress_transform_task)
else:
self.progress.start_task(task_id=self.overall_progress_download_task)
Expand All @@ -208,7 +215,11 @@ def start_task(self, task_id, task_type):

def advance(self, task_id, task_type):
if self.display_progress and self.overall_progress:
if task_type == "Transform":
if self.progress_counts[task_id].completed is not None:
self.progress_counts[task_id].completed += 1
else:
self.progress_counts[task_id].completed = 1
if task_type.endswith("Transform"):
self.progress.advance(task_id=self.overall_progress_transform_task)
else:
self.progress.advance(task_id=self.overall_progress_download_task)
Expand Down
25 changes: 23 additions & 2 deletions servicex/servicex_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,19 @@
from make_it_sync import make_sync
from servicex.databinder_models import ServiceXSpec, General, Sample
from collections.abc import Sequence
from enum import Enum
import traceback

T = TypeVar("T")
logger = logging.getLogger(__name__)


class ProgressBarFormat(str, Enum):
default = "default"
compact = "compact"
ponyisi marked this conversation as resolved.
Show resolved Hide resolved
none = "none"


class ReturnValueException(Exception):
"""An exception occurred at some point while obtaining this result from ServiceX"""

Expand Down Expand Up @@ -213,6 +220,7 @@ def deliver(
return_exceptions: bool = True,
fail_if_incomplete: bool = True,
ignore_local_cache: bool = False,
progress_bar: ProgressBarFormat = ProgressBarFormat.default,
):
config = _load_ServiceXSpec(config)

Expand All @@ -224,12 +232,25 @@ def deliver(

group = DatasetGroup(datasets)

if progress_bar == ProgressBarFormat.default:
progress_options = {}
elif progress_bar == ProgressBarFormat.compact:
progress_options = {"overall_progress": True}
elif progress_bar == ProgressBarFormat.none:
progress_options = {"display_progress": False}
else:
raise ValueError(f"Invalid value {progress_bar} for progress_bar provided")

if config.General.Delivery == General.DeliveryEnum.URLs:
results = group.as_signed_urls(return_exceptions=return_exceptions)
results = group.as_signed_urls(
return_exceptions=return_exceptions, **progress_options
)
return _output_handler(config, datasets, results)

elif config.General.Delivery == General.DeliveryEnum.LocalCache:
results = group.as_files(return_exceptions=return_exceptions)
results = group.as_files(
return_exceptions=return_exceptions, **progress_options
)
return _output_handler(config, datasets, results)


Expand Down
43 changes: 43 additions & 0 deletions tests/test_databinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,49 @@ def test_generic_query(codegen_list):
)


def test_deliver_progress_options(transformed_result, codegen_list):
from servicex import deliver, ProgressBarFormat
from servicex.query import UprootRaw # type: ignore

spec = ServiceXSpec.model_validate(
{
"Sample": [
{
"Name": "sampleA",
"RucioDID": "user.ivukotic:user.ivukotic.single_top_tW__nominal",
"Query": UprootRaw([{"treename": "nominal"}]),
}
]
}
)
with (
patch(
"servicex.dataset_group.DatasetGroup.as_files",
return_value=[transformed_result],
),
patch(
"servicex.servicex_client.ServiceXClient.get_code_generators",
return_value=codegen_list,
),
):
deliver(
spec,
config_path="tests/example_config.yaml",
progress_bar=ProgressBarFormat.compact,
)
deliver(
spec,
config_path="tests/example_config.yaml",
progress_bar=ProgressBarFormat.none,
)
with pytest.raises(ValueError):
deliver(
spec,
config_path="tests/example_config.yaml",
progress_bar="garbage",
)


def test_entrypoint_import():
"""This will check that we have at least the Python transformer defined in servicex.query"""
from servicex.query import PythonFunction # type: ignore # noqa: F401
13 changes: 9 additions & 4 deletions tests/test_expandable_progress.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def test_get_renderables_with_failure():

def test_progress_advance():
with ExpandableProgress() as progress:
t_id = progress.add_task("Transformation", True, 100)
t_id = progress.add_task("Transform", True, 100)
d_id = progress.add_task("Download", True, 100)
completed = 12
total = 100
Expand All @@ -173,6 +173,11 @@ def test_progress_advance():
d_id = progress.add_task("Download", True, 100)
completed = 12
total = 100
progress.update(d_id, "Transform", total, completed)
progress.advance(d_id, "Transform")
assert progress.progress.tasks[0].completed - 1 == completed
progress.update(d_id, "Download", total, completed)
progress.advance(d_id, "Download")
assert progress.progress.tasks[1].completed - 1 == completed

with ExpandableProgress(overall_progress=True) as progress:
d_id = progress.add_task("Download", True, 100)
progress.advance(d_id, "Download")
assert progress.progress.tasks[1].completed == 1
Loading