Opened 6 years ago
Closed 6 years ago
#17155 closed enhancement (fixed)
add "sage -installed" and "sage -pip" commands
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | scripts | Keywords: | |
Cc: | tmonteil | Merged in: | |
Authors: | Vincent Delecroix | Reviewers: | Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | 9810c8c (Commits) | Commit: | 9810c8cbf207320cf607c9769b713c29de678112 |
Dependencies: | Stopgaps: |
Description
Add two commands to sage:
sage -installed
to get the list of installed packagessage -pip
to run the Python package manager
Change History (20)
comment:1 Changed 6 years ago by
- Branch set to u/vdelecroix/17155
- Commit set to 4bf23308777288b5a829b18b153367ea0b7a2250
- Status changed from new to needs_review
comment:2 Changed 6 years ago by
Huh - and the pip would be quite useful. But even in the chunk you show,
if [ "$1" = '-i' ]; then shift # If there are no further arguments, simply list all installed
This should even be documented somewhere, but maybe it hasn't been advertised well lately? Not that it would be horrible to add this alias, though I don't know if some syntax warriors will complain because you allow -installed
while --installed
is the current vogue...
comment:3 Changed 6 years ago by
Hello,
Thanks for having a look.
I do not complain that sage -i
has not been advertised. But I do that it is documented nowhere (neither in src/bin/sage
nor in src/bin/sage-pkg
)!
Currently we have the options -optional
, -standard
and -experimental
. I am just adding an additional -installed
. We can be smarter and more coherent by allowing multiple arguments and force to use the -i
option like in
$ sage -i --installed --optional --standard ... would list all installed package that are either ... ... optional or standard ... $ sage -i --not-installed ... would list all non installed packages ...
I am in favour of a more coherent scheme of options, but then we should forbid the older syntax (and instead write a deprecation message that explains the new syntax).
And I definitely do not understand why we do have two scripts (src/bin/sage-spkg
and src/bin/sage-list-packages
) for dealing with packages. Morever the current state is that sage-spkg
is used to list the installed packages while sage-list-packages
is the one to list the optional packages...
EDIT: and I just noticed that the sage -i
starts with a useless line 'Currently installed packages:'. It is especially bad if you want to use it in other scripts.
Vincent
comment:4 Changed 6 years ago by
And I definitely do not understand why we do have two scripts
Alas, I can only point things out when it comes to these scripts - I am reluctant to tread on such long-standing traditions. I do hope some others come to look at this, though, since at the very least it is not bad to add -installed
.
comment:5 follow-up: ↓ 8 Changed 6 years ago by
- Status changed from needs_review to needs_work
Use spaces, not TABs for indentation.
Instead of [ ! $(sage -installed | grep -c '^pip-') -eq 1 ]
,
I would go with [ -x "$SAGE_LOCAL/bin/pip" ]
And why not replace echo "Pip is not installed. Run \"sage -i pip\" to install it."
by sage -i pip || exit $?
?
comment:6 Changed 6 years ago by
- Commit changed from 4bf23308777288b5a829b18b153367ea0b7a2250 to 6ae094b15c7e966a38ca92a280b7974b83c530c7
Branch pushed to git repo; I updated commit sha1. New commits:
6ae094b | trac #17155: reviewer comments
|
comment:7 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:8 in reply to: ↑ 5 Changed 6 years ago by
Replying to jdemeyer:
Use spaces, not TABs for indentation.
Instead of
[ ! $(sage -installed | grep -c '^pip-') -eq 1 ]
, I would go with[ -x "$SAGE_LOCAL/bin/pip" ]
done. It is much simpler, thanks.
And why not replace
echo "Pip is not installed. Run \"sage -i pip\" to install it."
bysage -i pip || exit $?
?
I do not like to do things in a script that users did not ask for.
Vincent
comment:9 Changed 6 years ago by
- Commit changed from 6ae094b15c7e966a38ca92a280b7974b83c530c7 to 0ded194dd551dc9a29b1991e0b782e53faa770da
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
0ded194 | trac #17155: reviewer comments
|
comment:10 Changed 6 years ago by
- Status changed from needs_review to needs_work
If you deprecate something, it should still work (i.e. don't remove the old code, just add the echo
statements).
Also, please use echo >&2
for these warnings.
comment:11 Changed 6 years ago by
- Reviewers set to Jeroen Demeyer
In --help
, you write -installed
while only --installed
works. I think you should stick to the de facto convention that both single and double dashes should work.
comment:12 Changed 6 years ago by
- Commit changed from 0ded194dd551dc9a29b1991e0b782e53faa770da to 31c2fec6204031e9882d278eb2ecd79bbdf74569
Branch pushed to git repo; I updated commit sha1. New commits:
31c2fec | trac #17155: nicer deprecation
|
comment:13 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:14 Changed 6 years ago by
- Status changed from needs_review to needs_work
In sage-list-packages
:
"standard
->"standard"
except A, B
->except A as B
- The
except
clause should abort the program otherwise one getsTraceback (most recent call last): File "/usr/local/src/sage-config/src/bin/sage-list-packages", line 80, in <module> available_version = dict([ split_pkgname(pkg) for pkg in get_remote_package_list(category)]) TypeError: 'int' object is not iterable
comment:15 Changed 6 years ago by
- Commit changed from 31c2fec6204031e9882d278eb2ecd79bbdf74569 to 9810c8cbf207320cf607c9769b713c29de678112
Branch pushed to git repo; I updated commit sha1. New commits:
9810c8c | trac #17155: reviewer comments
|
comment:16 Changed 6 years ago by
- Status changed from needs_work to needs_review
Thanks for your comments. I did all the modifications in my last commit.
comment:17 Changed 6 years ago by
ping!
comment:18 Changed 6 years ago by
- Status changed from needs_review to positive_review
comment:19 Changed 6 years ago by
Thanks.
comment:20 Changed 6 years ago by
- Branch changed from u/vdelecroix/17155 to 9810c8cbf207320cf607c9769b713c29de678112
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
trac #17155: add "sage -installed" and "sage -pip" commands