-
Notifications
You must be signed in to change notification settings - Fork 269
Stop defining simpleToString to be toString in robust.m2 #4073
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
base: development
Are you sure you want to change the base?
Conversation
simpleToString is our fallback when robust printing times out. But if we redefine it to be toString, then we could potentially call code that takes a long time to run, but it isn't interrupted.
|
Hmmm can you think of any reason why simpleToString was set to toString? I'm a little worried about breaking something we don't understand. Given that the I agree that this bounds the wait time in #3848 but it's not a fix. |
|
Also, I'm a little perplexed that |
|
Oops sorry misclick! |
|
I can't think of any reason it's been that way, but that's the way it's been since I'm not convinced we need more anything more than just |
|
Do you want to change it in this PR or later? |
If net (or Format, if it exists) fails or times out, then just jump straight to the fallback simpleToString rather than trying something else.
|
Might as well -- just pushed a commit removing the |
|
Hmm I keep going forth and back between just net > simpleToString and net > toString > simpleToString. I'm fine with this PR, but I'd rather have someone else also approve before merging. |
simpleToStringis our fallback when robust printing times out. But if we redefine it to betoString, then we could potentially call code that takes a long time to run, but it isn't interrupted.This fixes the second piece of #4069:
It also kind of fixes, or at least makes not as bad, #3848. We're still processing the error message, but it doesn't take as long.
This is because after giving
netandtoExternalString3 seconds each, we just fall back ontoSimpleStringnow and get a very basic error message: