Skip to content

[MRG] Added message on keyboard interrupt.#7614

Merged
lesteve merged 2 commits intoscikit-learn:masterfrom
kgilliam125:add-interrupt-message
Oct 20, 2016
Merged

[MRG] Added message on keyboard interrupt.#7614
lesteve merged 2 commits intoscikit-learn:masterfrom
kgilliam125:add-interrupt-message

Conversation

@kgilliam125
Copy link
Copy Markdown
Contributor

@kgilliam125 kgilliam125 commented Oct 8, 2016 •
edited by lesteve
Loading

Reference Issue

Fix #7596

What does this implement/fix? Explain your changes.

Added message to user that training has been interrupted.

Any other comments?

Anything else that should happen besides alerting the user that training was stopped?

@kgilliam125 kgilliam125 changed the title Added message on keyboard interrupt. [WIP] Added message on keyboard interrupt. Oct 8, 2016
@amueller
Copy link
Copy Markdown
Member

amueller commented Oct 8, 2016

The main question is whether we want a print or a warning. I think a print is fine. @jnothman any opinions? @denis-bz ?

@kgilliam125
Copy link
Copy Markdown
Contributor Author

kgilliam125 commented Oct 8, 2016

For what it's worth, I think print makes more sense than warning in this case since it is an explicit action (Keyboard Interrupt) that was taken by the user. I would be in favor of a warning if training were interrupted in a more programmatic way so that the user code could respond more appropriately.

@jnothman
Copy link
Copy Markdown
Member

jnothman commented Oct 8, 2016

I'm trained to believe that print to STDOUT without an option is always the wrong thing...

@kgilliam125
Copy link
Copy Markdown
Contributor Author

kgilliam125 commented Oct 13, 2016

I have no problem changing it if that's the desired behavior. I'll leave it up to you guys to decide what you want to happen.

@amueller
Copy link
Copy Markdown
Member

amueller commented Oct 14, 2016

I'm ok with warning.

@kgilliam125
Copy link
Copy Markdown
Contributor Author

kgilliam125 commented Oct 14, 2016

Sounds good. I'll make the change this afternoon when I update another PR.

@kgilliam125
Copy link
Copy Markdown
Contributor Author

kgilliam125 commented Oct 15, 2016

It seems UserWarning is an appropriate category for this, so I left it as the default. Thoughts?

@kgilliam125 kgilliam125 changed the title [WIP] Added message on keyboard interrupt. [MRG] Added message on keyboard interrupt. Oct 15, 2016
@amueller
Copy link
Copy Markdown
Member

amueller commented Oct 17, 2016

seems good to me.

@jnothman jnothman added this to the 0.18.1 milestone Oct 20, 2016
@lesteve
Copy link
Copy Markdown
Member

lesteve commented Oct 20, 2016

Merging this one, thanks a lot!

@lesteve lesteve merged commit cd714b1 into scikit-learn:master Oct 20, 2016
@kgilliam125 kgilliam125 deleted the add-interrupt-message branch October 20, 2016 20:43
amueller pushed a commit to amueller/scikit-learn that referenced this pull request Oct 25, 2016
@kgilliam125 @amueller
[MRG] Added warning on keyboard interrupt during MLP fit (scikit-lear…
e4dd9ae 
…n#7614)
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this pull request Jun 14, 2017
@kgilliam125 @Sundrique
[MRG] Added warning on keyboard interrupt during MLP fit (scikit-lear…
681d1a8 
…n#7614)
paulha pushed a commit to paulha/scikit-learn that referenced this pull request Aug 19, 2017
@kgilliam125 @paulha
[MRG] Added warning on keyboard interrupt during MLP fit (scikit-lear…
b365190 
…n#7614)
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this pull request Nov 15, 2017
@kgilliam125 @maskani-moh
[MRG] Added warning on keyboard interrupt during MLP fit (scikit-lear…
b292bd4 
…n#7614)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.18.1

Development

Successfully merging this pull request may close these issues.

except KeyboardInterrupt: pass in neural_network/multilayer_perceptron.py

4 participants

@kgilliam125 @amueller @jnothman @lesteve