ipython unnecessarily relies on sage-location

[sagetrac-felixs]

The interpreter line exchange for ipython should be done within spkg-install. The hack cannot be removed from sage-location otherwise.

Depends on #15146

CC: @jondo

Component: packages: standard

Branch/Commit: u/felixs/ipython @ 9c37286

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/15100

[jdemeyer]

comment:4

The "hack" you mention has nothing at all to do with ipython. What sage-location does affects every Python script in $SAGE_LOCAL/bin, so I don't see what this patch is trying to accomplish.

[sagetrac-felixs]

comment:5

I don't fully understand. In which way does sage-location have nothing to do with ipython if it rewrites the first line of ipython?

What else needs to be done in your opinion?

[jdemeyer]

comment:6

Replying to @sagetrac-felixs:

I don't fully understand. In which way does sage-location have nothing to do with ipython if it rewrites the first line of ipython?

It has nothing to do with ipython because $SAGE_LOCAL/bin/ipython is only one of the many scripts which are rewritten by sage-location.

Perhaps you should explain why the "hack" in sage-location should be removed.

[sagetrac-felixs]

comment:7

Every hack should be removed. Expecially the ones that implement things that can be achieved easily without hacks.

Hacks are often misleading and complicate issues. Getting rid of them needs more work, but this ticket is meant to just handle this particular case, I came across.

The whole story is: a doctest checks the first line in $SAGE_LOCAL/bin/ipython. This doctest will certainly fail, if ipython has been disabled (toplevel build system, #14796). That's why #14796, depends on #15105 (unhardwire paths). In the ipython case, unhardwiring paths can be simply achieved by removing the doctest. With this ticket, there is no doubt about how the first line in ipython looks like (in case it came from sage) and the doctest is unnecessary.

[jdemeyer]

comment:8

Replying to @sagetrac-felixs:

Every hack should be removed. Expecially the ones that implement things that can be achieved easily without hacks.

Fine, go ahead and implement sage-location without hacks...

By special-casing ipython you are achieving absolutely nothing: the "hack" in sage-location still exists and you're just fooling the doctest.

[jdemeyer]

comment:9

Replying to @sagetrac-felixs:

The whole story is: a doctest checks the first line in $SAGE_LOCAL/bin/ipython.

Sure, but even this doctest has nothing to do with ipython. It is only meant to check that sage-location worked.

[sagetrac-felixs]

comment:10

Replying to @jdemeyer:

Fine, go ahead and implement sage-location without hacks...

This is one part of it. Tickets must be achievable.

By special-casing ipython you are achieving absolutely nothing: the "hack" in sage-location still exists and you're just fooling the doctest.

I have fooled a bogus doctest. That's fine with me. What else do you suggest?

[sagetrac-felixs]

Dependencies: #15146

[jdemeyer]

comment:12

I still don't understand the point of this ticket. I think you should better explain what the problem is and why your patch fixes that problem.

[sagetrac-felixs]

comment:13

sage-location is meant to rewrite hardwired paths that contain $SAGE_OLD_ROOT, which is a sort of workaround (or, in the unconditional case, just "hack"). ipython does not depend on $SAGE_ROOT and there is no point in requiring this mechanism. Moreover, if spkg-install is self-contained, somebody might be able to read it and deduce how ipython on sage is installed and intended to work.

(I agree that there are worse issues, but this one is particularly easy to fix.)

[jdemeyer]

comment:14

there is no point in requiring this mechanism

Depends what you mean. The path-rewriting is needed to be able to relocate the Sage install tree.

Let me repeat this: By special-casing ipython you are achieving absolutely nothing: the "hack" in sage-location still exists and you're just fooling the doctest.

A proper fix for the "hack" would fix all Python scripts, not just this one. What's the point of this special case?

[sagetrac-felixs]

comment:15

Replying to @jdemeyer:

there is no point in requiring this mechanism

Depends what you mean. The path-rewriting is needed to be able to relocate the Sage install tree.

What i mean is, there is no point in requiring the mechanism for ipython. Most (maybe not all) cases are like this.

Let me repeat this: By special-casing ipython you are achieving absolutely nothing: the "hack" in sage-location still exists and you're just fooling the doctest.

there's a proper doctest in #15146, that does not rely on a wrong ipython installation.

A proper fix for the "hack" would fix all Python scripts, not just this one.

Yes.

What's the point of this special case?

I cannot fix all at once. But before the line has been merged, I will certainly not paste it into the other spkg-install files.

[jdemeyer]

comment:16

Replying to @sagetrac-felixs:

What's the point of this special case?

I cannot fix all at once.

You just gave a very good argument for keeping the sage-location "hack": it does rewrite all the paths at once without trouble. What's wrong with that?

I think you still need to explain what the problem is that this ticket is trying to fix, I really don't understand it.

[sagetrac-felixs]

comment:17

Replying to @jdemeyer:

Replying to @sagetrac-felixs:

What's the point of this special case?

I cannot fix all at once.

You just gave a very good argument for keeping the sage-location "hack":

No, I'm trying to make Sage a sane place. Something like this takes time.

it does rewrite all the paths at once without trouble. What's wrong with that?

It doesn't do anything useful in most cases. If there were no doctest looking at ipython i wouldn't even have noticed. A hack should always be restricted to things that require it. This is pretty similar to the problem about static atlas libraries (#15045): Its always better to not break/complicate things for all users just because one CPU/platform is broken/limitied/buggy.

I think you still need to explain what the problem is that this ticket is trying to fix, I really don't understand it.

Here we go, a more formal argument. read http://en.wikipedia.org/wiki/Encapsulation_(object-oriented_programming). There is a file that takes care of the contents of a package (spkg-install). Nothing beyond that is required. Other routines/programs that mess wich package contents, like sage-location does, violate encapsulation. Encapsulation, in general, is useful to keep things comprehensible and simple.

[jdemeyer]

comment:22

I think we can close this as duplicate (sort of) of #15146.

[jdemeyer]

Changed author from Felix Salfelder to none

[jdemeyer]

Reviewer: Jeroen Demeyer