-
-
Notifications
You must be signed in to change notification settings - Fork 545
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 sage_getfile by looking at __init__ #39499
Conversation
Documentation preview for this PR (built with commit 6aee498; changes) is ready! 🎉 |
You could use |
I copy the general structure from sage_getsourcelines . Presumably so that the attribute access only need to be done once ( Advantage is it would be 4 lines shorter I suppose. |
Thanks. For me it's just much easier to read when the logic is "if thing is true, do stuff that depends on thing." |
Otherwise it will fail with meson editable install on objects like `x` of type `Expression`, where the class does not have a docstring but `__init__` method have. The strategy is similar to `sage_getsourcelines` where `__init__` method is looked in. I choose to implement this instead of the more complex workaround (see the comment below). This fixes the test that fails in meson editable mode: ``` 2025-02-11T20:13:36.4801998Z ********************************************************************** 2025-02-11T20:13:36.4803076Z File "src/sage/misc/sageinspect.py", line 1363, in sage.misc.sageinspect.sage_getfile_relative 2025-02-11T20:13:36.4804104Z Failed example: 2025-02-11T20:13:36.4804750Z sage_getfile_relative(x) # needs sage.symbolic 2025-02-11T20:13:36.4808395Z Expected: 2025-02-11T20:13:36.4813350Z 'sage/symbolic/expression.pyx' 2025-02-11T20:13:36.4814244Z Got: 2025-02-11T20:13:36.4815714Z '/home/runner/work/sage/sage/builddir/src/sage/symbolic/expression.pyx' 2025-02-11T20:13:44.3128543Z ********************************************************************** ``` This may still fail in another case where neither class nor `__init__` method has a docstring (in that case `?` will fail to get the file in meson editable mode), but that's not in scope I guess. (I left a comment there to explain) See also sagemath#39369 ### 📝 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. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. (change should be invisible to users not using meson editable so…) ### ⌛ 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#39499 Reported by: user202729 Reviewer(s):
Otherwise it will fail with meson editable install on objects like
x
of typeExpression
, where the class does not have a docstring but__init__
method have.The strategy is similar to
sage_getsourcelines
where__init__
method is looked in.I choose to implement this instead of the more complex workaround (see the comment below).
This fixes the test that fails in meson editable mode:
This may still fail in another case where neither class nor
__init__
method has a docstring (in that case?
will fail to get the file in meson editable mode), but that's not in scope I guess. (I left a comment there to explain)See also #39369
📝 Checklist
⌛ Dependencies