-
-
Notifications
You must be signed in to change notification settings - Fork 567
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
Improve interface of list_packages #31385
Comments
Reviewer: Matthias Koeppe, ... |
Commit: |
Changed author from Tobias Diez to Tobias Diez, Matthias Koeppe |
comment:3
Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date. |
comment:4
Still needs review. Matthias, I'm happy with your changes - if that is sufficient, feel free to set it to positive review. |
comment:8
Replying to @dimpase:
With this plus #32036, the Sage library fails to build for me:
(I don't think that #31577 is involved.) |
comment:9
Replying to @jhpalmieri:
Or maybe it is, on another machine I'm seeing the same error with #31577 + #31385, but not #32036. |
comment:10
Maybe these previous posts are red herrings, and the problem is this ticket doesn't work with the latest beta. |
comment:11
Thanks for testing it -- I'll revisit it after the next beta |
comment:12
The following seems to fix it for me, but I don't know if it's a good and/or safe change: diff --git a/src/sage_setup/optional_extension.py b/src/sage_setup/optional_extension.py
index 06586f1d39..f252849384 100644
--- a/src/sage_setup/optional_extension.py
+++ b/src/sage_setup/optional_extension.py
@@ -48,7 +48,7 @@ def is_package_installed_and_updated(pkg):
# Might be an installed old-style package
condition = is_package_installed(pkg)
else:
- condition = (pkginfo["installed_version"] == pkginfo["remote_version"])
+ condition = (pkginfo.installed_version == pkginfo.remote_version)
return condition
|
comment:13
Setting a new milestone for this ticket based on a cursory review. |
Changed branch from u/mkoeppe/improve_interface_of_list_packages to u/jhpalmieri/improve_interface_of_list_packages |
comment:15
If my changes are okay, then I think we should merge it. New commits:
|
Changed reviewer from Matthias Koeppe, ... to Matthias Koeppe, John Palmieri |
Changed author from Tobias Diez, Matthias Koeppe to Tobias Diez, Matthias Koeppe, John Palmieri |
comment:16
They are fine with me. Setting this to positive review as it seems that everyone reviewed everyone else's changes. |
Changed reviewer from Matthias Koeppe, John Palmieri to Matthias Koeppe, John Palmieri, Tobias Diez |
comment:17
Yes, looks good to me. Thanks! |
Changed branch from u/jhpalmieri/improve_interface_of_list_packages to |
comment:19
|
Changed commit from |
(split out from #31013)
... representing package info using a new
PackageInfo
class, which provides also dict-like access, maintaining compatibility with the previous interface in this way.CC: @jhpalmieri @dimpase
Component: misc
Author: Tobias Diez, Matthias Koeppe, John Palmieri
Branch:
ebfb13c
Reviewer: Matthias Koeppe, John Palmieri, Tobias Diez
Issue created by migration from https://trac.sagemath.org/ticket/31385
The text was updated successfully, but these errors were encountered: