Conversation
8da18d6 to
99c45e8
Compare
|
I'd say |
|
The current version seems to work fine to launch sage, but fails at exit. Running and trying with and displays the banner again. Pressing |
|
Documentation preview for this PR (built with commit 1a872e3; changes) is ready! 🎉 |
+1 Also +100 on cleaning up the scripts I agree so it seems the paths are not quite right, and also the build paths are leaking. Similar experiment using the Build paths also leak on introspection. For example, if one does OTOH, something like |
src/sage/misc/banner.py
Outdated
| from sage.misc.superseded import deprecation | ||
|
|
||
| deprecation(1, "Use sage.version instead.") |
src/sage/cli/__init__.py
Outdated
| if __name__ == "__main__": | ||
| sys.exit(main()) |
There was a problem hiding this comment.
I think this needs to go in __main__.py so that python -m sage.cli works.
| if not input_args: | ||
| InteractiveShellCmd(CliOptions()).run() | ||
|
|
There was a problem hiding this comment.
| if not input_args: | |
| InteractiveShellCmd(CliOptions()).run() |
|
If you can fix the 2 or 3 suggestions above, we can get this in even if it's still missing critical functionality so the new package is usable. Changes suggested:
|
Took me a bit longer than expected to come back to this, sorry. The two options The other mentioned issues (exit issues, |
Thanks, this is looking good.
a) As much as I agree with on the need to move to standard tooling (like pytest), we have 20 years of doctests, they are not going to be moved to pytest any time soon. b) Devs are users, in fact the main users from my pov. c) Here's one important usage: I can write a script in a single $ sage test.sage
[runs the script]and my script can have functions with doctests and I can easily test them with $ sage -t test.sage
[runs the tests]This works with the current
Nice. I'll make some comments on the code later, but this is looking good. I am still using |
it's not that anything needs to be moved.
we can make |
dimpase
left a comment
There was a problem hiding this comment.
LGTM. Please switch this to Positive review
|
the failing isn't relevant here as far as I can see. |
sagemathgh-39015: Meson: add sage cli <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Meson currently doesn't install any of the scripts in `src/bin`. This is because - they mostly don't make sense for meson - especially doctests, sage packages interactions and various other developer tools shouldn't be installed for normal users - they rely on tricky environment variable manipulation - they use bash and thus do not work on Windows Here, we reimplement a very small subset of the sage cli functionality in Python without any env hacks. At the moment only `--version`, the interactive sage shell, `--notebook` and `-c` are implemented ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39015 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, Gonzalo Tornaría
|
The |
|
Based on a quick look, I would say in
The disadvantage here would be that one doesn't get a meaningful error if one mistypes a command (e.g. |
|
It would be good to still have a way to trigger the simple mode from the command line. This also affects some third-party UIs (eg. KDE's cantor [1]), which communicate with the sage process via terminal output and thus need to disable all coloring/banner/etc. Currently there's no clear way how to achieve that with meson-built sage. |
|
Another issue: the missing I will open an issue to track all test suite regression in the meson build, I assume these aren't caught by the CI because the executables are still present in the source tree. |
Done: #39872 |
|
in the context of notebooks, it's not clear to me what |
|
The wording is perhaps not the best, but a general reference to the module |
|
This is what I see in Sage 10.8beta6: and I'm confused what is the suggested replacement for Can this |
|
This is what I see in Sage 10.8beta6:
```
sage: version()
<ipython-input-28-e742030be3c2>:1: DeprecationWarning: Use sage.version instead.
See #39015 for details.
version()
'SageMath version 10.8.beta6, Release Date: 2025-10-06'
sage: sage.version
<module 'sage.version' from '/usr/local/sage/src/sage/version.py'>
```
and I'm confused what is the suggested replacement for `version()`.
```
sage: sage.version.version
'10.8.beta6'
```
works. the message has to be corrected.
…
Can this `DeprecationWarning` be made more clear?
|
sagemathgh-41050: fix deprecation message in banner.py this version actually does work. See also sagemath#39015 (comment) <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41050 Reported by: Dima Pasechnik Reviewer(s): Max Alekseyev, Tobias Diez
sagemathgh-41050: fix deprecation message in banner.py this version actually does work. See also sagemath#39015 (comment) <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41050 Reported by: Dima Pasechnik Reviewer(s): Max Alekseyev, Tobias Diez
sagemathgh-41050: fix deprecation message in banner.py this version actually does work. See also sagemath#39015 (comment) <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41050 Reported by: Dima Pasechnik Reviewer(s): Max Alekseyev, Tobias Diez
sagemathgh-41050: fix deprecation message in banner.py this version actually does work. See also sagemath#39015 (comment) <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#41050 Reported by: Dima Pasechnik Reviewer(s): Max Alekseyev, Tobias Diez
|
@antonio-rojas Hi. I recently installed Sage 10.7 in ArchLinux WSL2. However, I get the following problem after clean install: This worked like a charm for Ubuntu installation ( I was suggested that a revert to use the old bash script should be done, until the new meson-based CLI has compatible functionality. Is this something you can take care of? Thanks in advance. |
There's nothing to implement, the old |
|
I suggested that one. That said, making but more generally: we have a deprecation policy such that features cannot be removed before one year of deprecation. It is not clear that policy applies only to "Python classes or functions without underscore in its name" (*) or also to other utilities, but if the latter were the case, the correct course of action would be to add it back (possibly with deprecation warnings). (*): from https://doc.sagemath.org/html/en/developer/coding_in_python.html#deprecation
I can't find anything that discuss backwards incompatible change to utilities outside |

Meson currently doesn't install any of the scripts in
src/bin. This is becauseHere, we reimplement a very small subset of the sage cli functionality in Python without any env hacks. At the moment only
--version, the interactive sage shell,--notebookand-care implemented📝 Checklist
⌛ Dependencies