Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow me to turn off progress notification #1264

Closed
koaning opened this issue Jan 30, 2025 · 9 comments · Fixed by #1334
Closed

Allow me to turn off progress notification #1264

koaning opened this issue Jan 30, 2025 · 9 comments · Fixed by #1334
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@koaning
Copy link
Contributor

koaning commented Jan 30, 2025

Describe the bug

Image

In this codeblock you can see that I am trying to wrap around report.metrics.metrics_report(). However, it always displays the progress bar. Can we turn this off? It makes sense when called directly but it is in the way when I try to build something that wraps around it.

Steps/Code to Reproduce

Just call .metrics.metrics_report on the cross validation report.

Expected Behavior

Have a flag to turn it off.

Actual Behavior

There is no such flag.

Environment

System:
    python: 3.12.6 (main, Sep  6 2024, 19:03:47) [Clang 15.0.0 (clang-1500.1.0.2.5)]
executable: /Users/vincent/Development/probabl/.venv/bin/python3
   machine: macOS-13.4.1-arm64-arm-64bit

Python dependencies:
        skore: 0.6.1
          pip: None
   setuptools: 75.8.0
    diskcache: 5.6.3
      fastapi: 0.115.7
       joblib: 1.4.2
   matplotlib: 3.10.0
        numpy: 2.2.2
       pandas: 2.2.3
       plotly: 5.24.1
      pyarrow: 19.0.0
         rich: 13.9.4
 scikit-learn: 1.6.1
        skops: 0.11.0
      uvicorn: 0.34.0
Selection deleted
@koaning koaning added bug Something isn't working needs-triage This has been recently submitted and needs attention labels Jan 30, 2025
@MarieSacksick MarieSacksick added enhancement New feature or request good first issue Good for newcomers and removed bug Something isn't working needs-triage This has been recently submitted and needs attention labels Jan 30, 2025
@ictorv
Copy link
Contributor

ictorv commented Jan 31, 2025

Will it works if we add flag show_progress to disable progress bar?
Refer: https://github.com/probabl-ai/skore/blob/main/skore/src/skore/sklearn/_estimator/metrics_accessor.py

def report_metrics(
    self,
    *,
    data_source="test",
    X=None,
    y=None,
    scoring=None,
    scoring_names=None,
    pos_label=None,
    scoring_kwargs=None,
    show_progress=True  # Parameter Here
):

And set progress context

progress_context = tqdm if show_progress else nullcontext

Then append into score inside report_metrics function

            if metric_name is not None :
                metrics_kwargs["metric_name"] = metric_name
                with progress_context():
                  scores.append(
                    metric_fn(
                       data_source=data_source,
                        X=X,
                        y=y,
                        **metrics_kwargs,
                  )

Then might

report.metrics.report_metrics(show_progress=False).to_html()

will be solved @MarieS-WiMLDS ?

@glemaitre
Copy link
Member

Maybe the change by @ictorv that we merge yesterday is actually resolving the problem spotted by @koaning: now the progress bars are transient, meaning that they disappear once the computation to stop taking vertical space that is indeed annoying and pollute the output.

Would it be enough @koaning or you are really do not want them?

@glemaitre
Copy link
Member

glemaitre commented Jan 31, 2025

If we need to explicitly turn them off even during processing, then I think that we should have a set_config and config_context such that we globally set instead of adding a parameter in the different function/class that will create such bars:

set_config(show_progress=False)  # deactivate the progress

report = CrossValidationReport(...)

# or as a context manager
with config_context(show_progress=False):
    report = CrossValidationReport(...)

Such config could be useful in the future for instance to return pandas vs. polars or choosing the backend plotting matplotlib vs. plotly.

@koaning
Copy link
Contributor Author

koaning commented Feb 3, 2025

If it goes away automatically now/soon then that can also work. The context manager could also work.

@sylvaincom
Copy link
Contributor

sylvaincom commented Feb 4, 2025

Thanks for the feedback. As discussed above, due to #1255, would it be ok to close this? cc @MarieS-WiMLDS

@glemaitre
Copy link
Member

I think we can keep it open to implement the context configuration.

@auguste-probabl
Copy link
Contributor

As a dev, the progress bar makes my work more difficult. For example when debugging with ipdb, my input gets hidden and replaced with the progress bar. My current solution is the apply a custom patch that removes all progress bar logic before starting work.

@glemaitre
Copy link
Member

I'm still up for implementing the configuration thingy.

@auguste-probabl what is the reason for the debugger to not work? I would think that the current implementation is just a decorator? Is it that your debugger get caught in the progress bar thread?

@auguste-probabl
Copy link
Contributor

Is it that your debugger get caught in the progress bar thread?

Yes, something like this

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants