#32331 closed enhancement (fixed)

refresh sage-fixdoctests using argparse

Reported by: chapoton Owned by:
Priority: major Milestone: sage-9.5
Component: scripts Keywords:
Cc: tscrim, slelievre, gh-kliem Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 7f4b0c0 (Commits, GitHub, GitLab) Commit: 7f4b0c0d4d422358559ee65fcc3fb46789b24a21
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

see also #31366 and #32332

also full flake8 cleanup of the modified file

Change History (10)

comment:1 Changed 11 months ago by chapoton

  • Branch set to u/chapoton/32331
  • Commit set to 7f4b0c0d4d422358559ee65fcc3fb46789b24a21
  • Status changed from new to needs_review

New commits:

7f4b0c0refresh sage-fixdoctests using argparse

comment:2 Changed 11 months ago by chapoton

  • Description modified (diff)

comment:3 Changed 11 months ago by chapoton

  • Cc tscrim slelievre gh-kliem added

comment:4 Changed 11 months ago by tscrim

  • Reviewers set to Travis Scrimshaw

One thing that could be considered a regression is that if I do something like

./sage --fixdoctests src/sage/combinat/q_analogues.py src/sage/combinat/rsk.py src/sage/combinat/fqsym.py

then I now get that rsk.py is an empty file, whereas the old behavior left rsk.py alone. So what we get is a new file with no output when the is erroneous input passed. There might not be a way around this because that is how argparse works, but I wanted to ask before giving the go-ahead with this.

comment:5 Changed 11 months ago by chapoton

ok, I see.

We could introduce an option {{{-o, --output}} and try to handle multiple input files.

Or stay with the principle that this is to fix just one single file ?

Version 0, edited 11 months ago by chapoton (next)

comment:6 Changed 11 months ago by tscrim

I think it is fine to handle just one file, which I believe is documented. It is more if someone gives it bad input (and it realizes this) that it doesn't change any files. However, this isn't really a big deal. I think the current branch is fine, but I wanted to see if there was an easy fix to prevent this from happening.

comment:7 Changed 11 months ago by chapoton

I do not see how to distinguish between the incorrect use

sage --fixdoctests a.py b.py

where the intention is to fix both a.py and b.py inplace ; and the correct use

sage --fixdoctests a.py b.py

where the intention is to write a corrected copy of a.py in b.py.

In the second case, it could very well be that b.py is existing already, as in the first case.

I think one can reasonably keep the current behaviour, which is indeed documented.

comment:8 Changed 11 months ago by tscrim

  • Status changed from needs_review to positive_review

I agree that we cannot distinguish between improper use and proper use. What I was saying is if someone does improper use like

sage --fixdoctests a.py b.py c.py

that b.py is not overwritten with a blank file (previously it did not). However, I doubt we can easily fix this, so I am setting this to a positive review.

comment:9 Changed 11 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:10 Changed 10 months ago by vbraun

  • Branch changed from u/chapoton/32331 to 7f4b0c0d4d422358559ee65fcc3fb46789b24a21
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.