Opened 6 years ago

Closed 6 years ago

#18110 closed enhancement (fixed)

Improve "unable to convert string" error message

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.6
Component: user interface Keywords: sd66
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: 1a1976c (Commits) Commit: 1a1976cf76b570e2f6d667f7a894ee9dea19f303
Dependencies: Stopgaps:

Description


Change History (9)

comment:1 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/improve__unable_to_convert_string__error_message

comment:2 Changed 6 years ago by jdemeyer

  • Commit set to abc5a70de3fd3007fc603fdf378164c4e31a3967
  • Keywords sd66 added
  • Status changed from new to needs_review

New commits:

​abc5a70Improve "unable to convert string" error message

comment:3 follow-up: Changed 6 years ago by kcrisman

Only one question: 

+        if isinstance(x, basestring):
+            from sage.misc.sage_eval import sage_eval

but there is no corresponding import for y, so in theory there could be some situation where x wasn't a basestring (whatever that is) but y was. ? Otherwise looks good.

comment:4 Changed 6 years ago by kcrisman

Otherwise all looks good and passes all tests in sage/rings.

comment:5 in reply to: ↑ 3 ; follow-up: Changed 6 years ago by jdemeyer

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to needs_work

Replying to kcrisman:

Only one question: 

+        if isinstance(x, basestring):
+            from sage.misc.sage_eval import sage_eval

but there is no corresponding import for y, so in theory there could be some situation where x wasn't a basestring (whatever that is) but y was. ?

Good catch. Interestingly, that error was caught by the except NameError:...

comment:6 Changed 6 years ago by git

  • Commit changed from abc5a70de3fd3007fc603fdf378164c4e31a3967 to 1a1976cf76b570e2f6d667f7a894ee9dea19f303

Branch pushed to git repo; I updated commit sha1. New commits:

​1a1976cImport sage_eval

comment:7 Changed 6 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:8 in reply to: ↑ 5 Changed 6 years ago by kcrisman

  • Status changed from needs_review to positive_review

Good catch.

Otherwise all is well.

Interestingly, that error was caught by the except NameError:...

LOL

comment:9 Changed 6 years ago by vbraun

  • Branch changed from u/jdemeyer/improve__unable_to_convert_string__error_message to 1a1976cf76b570e2f6d667f7a894ee9dea19f303
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.