#29179 closed defect (duplicate)

Small bug in PHC interface

Reported by: kcrisman Owned by:
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: interfaces Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

While trying to fix things on https://wiki.sagemath.org/interact/algebra I noticed the following problem if one doesn't have PHC installed:

sage: print(os.system('which phc') + '  PHC needs to be installed and in your path')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-f9a470112020> in <module>()
----> 1 print(os.system('which phc') + '  PHC needs to be installed and in your path')

TypeError: unsupported operand type(s) for +: 'int' and 'str'
sage: os.system('which phc') 
256

This is from https://github.com/sagemath/sage/blob/develop/src/sage/interfaces/phc.py#L939

        if e:
            from sage.misc.sage_ostools import have_program
            if not have_program('phc'):
                print(os.system('which phc') + '  PHC needs to be installed and in your path')
                raise RuntimeError
            # todo -- why? etc.
            with open(log_filename) as f:
                msg = f.read()
            raise RuntimeError(msg + "\nError running phc.")

But it is never tested because all the optional tests are, well, optional! Anyway, the fix should be pretty easy, make it a string - presumably something left over from the py3 switch.

But we should also test this, somehow. How do you test for when a package is not installed?

Change History (5)

comment:1 follow-up: Changed 14 months ago by chapoton

see #29163 ?

comment:2 in reply to: ↑ 1 Changed 14 months ago by kcrisman

  • Milestone changed from sage-9.1 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

see #29163 ?

Hilarious! I probably saw that email and said, "I don't know anything about PHC" and ignored it ... please accept my apologies, and thanks for fixing it!

comment:3 Changed 14 months ago by kcrisman

  • Status changed from needs_review to positive_review

comment:4 Changed 14 months ago by kcrisman

Though my comment about testing still stands ... is there some ticket open about testing for the non-presence of a package? This is a desideratum. I can open one if you don't know of one.

comment:5 Changed 14 months ago by chapoton

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.