Skip to content

Comments

doctester: Check for consistency of # optional annotations#35620

Merged
vbraun merged 7 commits intosagemath:developfrom
mkoeppe:doctest-linter
Jun 21, 2023
Merged

doctester: Check for consistency of # optional annotations#35620
vbraun merged 7 commits intosagemath:developfrom
mkoeppe:doctest-linter

Conversation

@mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented May 7, 2023

📚 Description

For example:

sage -t --long --random-seed=52927571004392838159040103596363721016 src/sage/rings/function_field/function_field.py
**********************************************************************
File "src/sage/rings/function_field/function_field.py", line 40, in sage.rings.function_field.function_field
Warning: Variable 'L' referenced here was set only in doctest marked '# optional - sage.rings.finite_rings sage.rings.function_field'
    S.<t> = L[]                                                                   # optional - sage.rings.finite_rings

Resolves #35401

📝 Checklist

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

⌛ Dependencies

kwankyu
kwankyu approved these changes May 9, 2023
View reviewed changes
Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works well.

I was a bit concerned that this would slowdown doctesting slightly, but the concern seems imaginary.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 9, 2023

Thank you!

@vbraun
Copy link
Member

vbraun commented May 27, 2023

Test fail

It seems that # long time tests are not covered by the CI, is that true?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 27, 2023

Test fail

Sorry about this, will fix.

It seems that # long time tests are not covered by the CI, is that true?

Yes, that's currently true.

#35652 changes this (by first running a fast incremental build/test, then running a full build of the Sage and ptest --long)

@vbraun
Copy link
Member

vbraun commented Jun 3, 2023

Tests still fail. Thats two strikes. Please don't make it three.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 12, 2023

Sorry about this. Fixed now.

@github-actions
Copy link

github-actions bot commented Jun 12, 2023

Documentation preview for this PR (built with commit ac1f1fe) is ready! 🎉

@vbraun vbraun merged commit 1c782f2 into sagemath:develop Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kwankyu kwankyu kwankyu approved these changes

Assignees

@mkoeppe mkoeppe

Labels

c: doctest framework

Projects

None yet

Milestone

sage-10.1

Development

Successfully merging this pull request may close these issues.

Doctest # optional / # long annotation dataflow checker

3 participants

@mkoeppe @vbraun @kwankyu