Skip to content

sage -fixdoctests: Handle directory names, call sage -t only once #36024

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 5, 2023
Merged

sage -fixdoctests: Handle directory names, call sage -t only once #36024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50baf64
src/bin/sage-fixdoctests: Do not act on 'NameError: x'
mkoeppe Jul 12, 2023
7302ff3
src/bin/sage-fixdoctests: Run doctester for all files in parallel, ha…
mkoeppe Jul 12, 2023
255e2d8
sage -fixdoctests: Handle ImportError too
mkoeppe Jul 17, 2023
f631de2
sage -t --if-installed: Ignore all files except .py and .pyx
mkoeppe Jul 15, 2023
fe79282
sage -fixdoctests: Fix handling of 2 file arguments
mkoeppe Aug 3, 2023
2dccf18
sage -fixdoctests: Suppress 'too few successful tests' message
mkoeppe Aug 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 83 additions & 44 deletions src/bin/sage-fixdoctests
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import sys
from argparse import ArgumentParser, FileType
from pathlib import Path

from sage.doctest.control import skipfile
from sage.doctest.control import DocTestDefaults, DocTestController
from sage.doctest.parsing import parse_file_optional_tags, parse_optional_tags, unparse_optional_tags, update_optional_tags
from sage.env import SAGE_ROOT
from sage.features import PythonModule
Expand Down Expand Up @@ -189,8 +189,12 @@ def process_block(block, src_in_lines, file_optional_tags):
return

# Error testing.
if m := re.search(r"ModuleNotFoundError: No module named '([^']*)'", block):
module = m.group(1)
if m := re.search(r"(?:ModuleNotFoundError: No module named|ImportError: cannot import name '([^']*)' from) '([^']*)'", block):
if m.group(1):
# "ImportError: cannot import name 'function_field_polymod' from 'sage.rings.function_field' (unknown location)"
module = m.group(2) + '.' + m.group(1)
else:
module = m.group(2)
asked_why = re.search('#.*(why|explain)', src_in_lines[first_line_num - 1])
optional = module_feature(module)
if optional and optional.name not in file_optional_tags:
Expand Down Expand Up @@ -224,6 +228,8 @@ def process_block(block, src_in_lines, file_optional_tags):
# NameError from top level, so keep it brief
if m := re.match("NameError: name '(.*)'", got[index_NameError:]):
name = m.group(1)
if name == 'x': # Don't mark it '# needs sage.symbolic'; that's almost always wrong
return
if feature := name_feature(name):
add_tags = [feature.name]
else:
Expand Down Expand Up @@ -305,17 +311,21 @@ def process_block(block, src_in_lines, file_optional_tags):


# set input and output files
if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite:
inputs, outputs = [args.filename[0]], [args.filename[1]]
print("sage-fixdoctests: When passing two filenames, the second one is taken as an output filename; "
"this is deprecated. To pass two input filenames, use the option --overwrite.")
elif args.no_overwrite:
inputs, outputs = args.filename, [input + ".fixed" for input in args.filename]
else:
inputs = outputs = args.filename
def output_filename(filename):
if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite:
if args.filename[0] == filename:
return args.filename[1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I get

$ sage -fixdoctests src/sage/rings/function_field/constructor.py output
Running "sage -t -p src/sage/rings/function_field/constructor.py output"
ERROR: file or directory not found: output

sage-runtests: Skipping 'output' because it does not have one of the recognized file name extensions
too few successful tests, not using stored timings
Skipping 'output' because it does not have one of the recognized file name extensions
No fixes made in 'src/sage/rings/function_field/constructor.py'

Shouldn't I get the deprecation warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for testing this case. I've made the necessary fixes.

else:
return None
return filename + ".fixed"
if args.no_overwrite:
return filename + ".fixed"
return filename

# Test the doctester, putting the output of the test into sage's temporary directory
if not args.no_test:
if args.no_test:
doc_out = ''
else:
executable = f'{os.path.relpath(args.venv)}/bin/sage' if args.venv else 'sage'
environment_args = f'--environment {args.environment} ' if args.environment != runtest_default_environment else ''
long_args = f'--long ' if args.long else ''
Expand All @@ -329,38 +339,61 @@ if not args.no_test:
if status := os.waitstatus_to_exitcode(os.system(f'{cmdline} > {shlex.quote(doc_file)}')):
print(f'Doctester exited with error status {status}')
sys.exit(status)
# Run the doctester, putting the output of the test into sage's temporary directory
if len(args.filename) == 2 and not args.overwrite and not args.no_overwrite:
print("sage-fixdoctests: When passing two filenames, the second one is taken as an output filename; "
"this is deprecated. To pass two input filenames, use the option --overwrite.")

for input, output in zip(inputs, outputs):
if (skipfile_result := skipfile(input, True, log=print)) is True:
continue

if args.no_test:
doc_out = ''
input_filenames = [args.filename[0]]
else:
# Run the doctester, putting the output of the test into sage's temporary directory
cmdline = f'{shlex.quote(executable)} -t {environment_args}{long_args}{probe_args}{lib_args}{shlex.quote(input)}'
print(f'Running "{cmdline}"')
os.system(f'{cmdline} > {shlex.quote(doc_file)}')
input_filenames = args.filename
input_args = " ".join(shlex.quote(f) for f in input_filenames)
cmdline = f'{shlex.quote(executable)} -t -p {environment_args}{long_args}{probe_args}{lib_args}{input_args}'
print(f'Running "{cmdline}"')
os.system(f'{cmdline} > {shlex.quote(doc_file)}')

with open(doc_file, 'r') as doc:
doc_out = doc.read()
with open(doc_file, 'r') as doc:
doc_out = doc.read()

# echo control messages
for m in re.finditer('^Skipping .*', doc_out, re.MULTILINE):
print('sage-runtests: ' + m.group(0))
break
else:
sep = "**********************************************************************\n"
doctests = doc_out.split(sep)

sep = "**********************************************************************\n"
doctests = doc_out.split(sep)

seen = set()

def block_filename(block):
if not (m := re.match('File "([^"]*)", line ([0-9]+), in ', block)):
return None
return m.group(1)

def expanded_filename_args():
DD = DocTestDefaults(optional='all', warn_long=10000)
DC = DocTestController(DD, input_filenames)
DC.add_files()
DC.expand_files_into_sources()
for source in DC.sources:
yield source.path

def process_grouped_blocks(grouped_iterator):

for input, blocks in grouped_iterator:

if not input: # Blocks of noise
continue
if input in seen:
continue
seen.add(input)

with open(input, 'r') as test_file:
src_in = test_file.read()
src_in_lines = src_in.splitlines()
shallow_copy_of_src_in_lines = list(src_in_lines)

file_optional_tags = set(parse_file_optional_tags(enumerate(src_in_lines)))

for block in doctests:
for block in blocks:
process_block(block, src_in_lines, file_optional_tags)

# Now source line numbers do not matter any more, and lines can be real lines again
Expand Down Expand Up @@ -392,20 +425,26 @@ for input, output in zip(inputs, outputs):
persistent_optional_tags = {}

if src_in_lines != shallow_copy_of_src_in_lines:
with open(output, 'w') as test_output:
for line in src_in_lines:
if line is None:
continue
test_output.write(line)
test_output.write('\n')

# Show summary of changes
if input != output:
print("The fixed doctests have been saved as '{0}'.".format(output))
if (output := output_filename(input)) is None:
print(f"Not saving modifications made in '{input}'")
else:
relative = os.path.relpath(output, SAGE_ROOT)
print(f"The input file '{output}' has been overwritten.")
if not relative.startswith('..'):
subprocess.call(['git', '--no-pager', 'diff', relative], cwd=SAGE_ROOT)
with open(output, 'w') as test_output:
for line in src_in_lines:
if line is None:
continue
test_output.write(line)
test_output.write('\n')
# Show summary of changes
if input != output :
print("The fixed doctests have been saved as '{0}'.".format(output))
else:
relative = os.path.relpath(output, SAGE_ROOT)
print(f"The input file '{output}' has been overwritten.")
if not relative.startswith('..'):
subprocess.call(['git', '--no-pager', 'diff', relative], cwd=SAGE_ROOT)
else:
print(f"No fixes made in '{input}'")

process_grouped_blocks(
itertools.chain(itertools.groupby(doctests, block_filename),
((filename, []) for filename in expanded_filename_args())))
4 changes: 4 additions & 0 deletions src/sage/doctest/control.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ def skipfile(filename, tested_optional_tags=False, *,
if log:
log(f"Skipping '{filename}' because it does not have one of the recognized file name extensions")
return True
if if_installed and ext not in ('.py', '.pyx'):
if log:
log(f"Skipping '{filename}' because it is not the source file of a Python module")
return True
if "jupyter_execute" in filename:
if log:
log(f"Skipping '{filename}' because it is created by the jupyter-sphinx extension for internal use and should not be tested")
Expand Down