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

Elapsed step times for inference stage in Pipeline and FeatureUnion #19293

Closed

Conversation

MezentsevIlya
Copy link

@MezentsevIlya MezentsevIlya commented Jan 28, 2021

Reference Issues/PRs

#19291

What does this implement/fix? Explain your changes.

Hello!

I think possibility to print elapsed time for steps on inference stage can be useful. Previously it was possible only for fitting stage. Now when parameter verbose is greater than or equal to 10 logs will be printed for both fitting and inference stages.

@MezentsevIlya MezentsevIlya changed the title Steps time for inference Steps time for inference in Pipeline and FeatureUnion Jan 29, 2021
@MezentsevIlya MezentsevIlya changed the title Steps time for inference in Pipeline and FeatureUnion Elapsed step times for inference stage in Pipeline and FeatureUnion Jan 29, 2021
@MezentsevIlya
Copy link
Author

@thomasjpfan, hi!
Sorry for disturbing. I see your activity lately. Maybe you can help me :)
It's my first PR, and it's already one week old. Do I need to call someone to review it or just wait?

@thomasjpfan
Copy link
Member

I think we need to discussion on the issue if we want to include this feature. I do not think we normally have printing during transform.

@MezentsevIlya
Copy link
Author

I think we need to discussion on the issue if we want to include this feature. I do not think we normally have printing during transform.

I think It's can be useful if you want to estimate how long your pipeline will be transform 10/100/100000 elements.

@jnothman
Copy link
Member

jnothman commented Feb 7, 2021

Without really considering the value of the feature, I wouldn't want verbosity in transform at least while we are printing to stdout (cf. #17439), since it might mess up scripting uses of stdout.

@glemaitre
Copy link
Member

I think that it is better to look at the API for proper logging instead of adding some verbosity for the moment.
We can put on hold this PR.

@glemaitre glemaitre closed this Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants