Skip to content
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

Spurious warnings about optional tags #36920

Closed
2 tasks done
kwankyu opened this issue Dec 19, 2023 · 4 comments · Fixed by #36950
Closed
2 tasks done

Spurious warnings about optional tags #36920

kwankyu opened this issue Dec 19, 2023 · 4 comments · Fixed by #36950

Comments

@kwankyu
Copy link
Collaborator

kwankyu commented Dec 19, 2023

Steps To Reproduce

From sage-devel thread

https://groups.google.com/g/sage-devel/c/kagz-697Hts


[David Coudert] observed some doctest warnings that I don't know how to fix. This is certainly a side effect of the modularization.

File "src/sage/plot/plot.py", line 569, in sage.plot.plot
Warning: Variable 'sig_on_count' referenced here was set only in doctest marked '# needs sage.symbolic'
    sig_on_count() # check sig_on/off pairings (virtual doctest)
**********************************************************************
File "src/sage/plot/plot.py", line 3623, in sage.plot.plot.graphics_array
Warning: Variable 'x' referenced here was set only in doctest marked '# long time, needs sage.symbolic'
    L = [plot(sin(k*x), (x,-pi,pi)) for k in [1..3]]
**********************************************************************
File "src/sage/plot/plot.py", line 3669, in sage.plot.plot.graphics_array
Warning: Variable 'x' referenced here was set only in doctest marked '# long time, needs sage.symbolic'
    p1 = plot(sin(x^2), (x, 0, 6),
              axes_labels=[r'$\theta$', r'$\sin(\theta^2)$'], fontsize=16)
**********************************************************************
File "src/sage/plot/plot.py", line 3671, in sage.plot.plot.graphics_array
Warning: Variable 'x' referenced here was set only in doctest marked '# long time, needs sage.symbolic'
    p2 = plot(x^3, (x, 1, 100), axes_labels=[r'$x$', r'$y$'],
              scale='semilogy', frame=True, gridlines='minor')

File "src/sage/repl/user_globals.py", line 52, in sage.repl.user_globals
Warning: Variable 'sig_on_count' referenced here was set only in doctest marked '# needs sage.modules'
    sig_on_count() # check sig_on/off pairings (virtual doctest)
**********************************************************************
File "src/sage/repl/user_globals.py", line 99, in sage.repl.user_globals.get_globals
Warning: Variable 'sig_on_count' referenced here was set only in doctest marked '# needs sage.modules'
    sig_on_count() # check sig_on/off pairings (virtual doctest)

An initial investigation suggests:

The string sig_on_count() # check sig_on/off pairings (virtual doctest) is from src/sage/doctest/sources.py line 234.
As I understand it, the warning is caused by a bug in the method _create_doctests() in the same file, which fails to add optional tags correctly to the string.

Expected Behavior

No warning.

Actual Behavior

Spurious warnings

Additional Information

@mkoeppe would know how to fix it.

Environment

- **OS**:
- **Sage Version**:

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@kwankyu kwankyu added the t: bug label Dec 19, 2023
@RuchitJagodara
Copy link
Contributor

Hey! I was trying to post on Google Groups, but it seems that I don't have permission to do so. Therefore, I am posting it here.

@dcoudert, could you please provide more information regarding reproducing this? It seems that there have been some changes because I just saw a file described in the first warning, which is src/sage/plot/plot.py. However, I couldn't find anything similar to sig_on_count, and the line described in that warning does not contain any code; it only includes the end of a comment block.

@dcoudert
Copy link
Contributor

As explained by @kwankyu above, this is a side effect of other mechanisms. I have no clue how to fix that.

@mkoeppe
Copy link
Contributor

mkoeppe commented Dec 19, 2023

sig_on_count comes from a "virtual doctest" that the doctest framework injects, and the doctest framework also directly manipulates the "user globals" mechanism.

I'll check if I have fixes for the warnings in sage.plot already in #35095.

@RuchitJagodara
Copy link
Contributor

Ok, thank you ! And if changes are needed then please tell me, I will be happy to help in this.

vbraun pushed a commit to vbraun/sage that referenced this issue Dec 24, 2023
…ix `sig_on_count` doctest dataflow warnings

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
Fixes sagemath#36920

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
In part cherry picked from:
- sagemath#35095
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] 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 accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
- Depends on sagemath#36905 (merged here)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36950
Reported by: Matthias Köppe
Reviewer(s): David Coudert
vbraun pushed a commit to vbraun/sage that referenced this issue Dec 26, 2023
…ix `sig_on_count` doctest dataflow warnings

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
Fixes sagemath#36920

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
In part cherry picked from:
- sagemath#35095
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] 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 accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
- Depends on sagemath#36905 (merged here)

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36950
Reported by: Matthias Köppe
Reviewer(s): David Coudert
@mkoeppe mkoeppe added this to the sage-10.3 milestone Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants