Opened 10 months ago
Closed 9 months ago
#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: |
Description (last modified by )
Change History (10)
comment:1 Changed 10 months ago by
- Branch set to u/chapoton/32331
- Commit set to 7f4b0c0d4d422358559ee65fcc3fb46789b24a21
- Status changed from new to needs_review
comment:2 Changed 10 months ago by
- Description modified (diff)
comment:3 Changed 10 months ago by
- Cc tscrim slelievre gh-kliem added
comment:4 Changed 10 months ago by
- 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 10 months ago by
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 ?
comment:6 Changed 10 months ago by
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 10 months ago by
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 10 months ago by
- 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 9 months ago by
- Milestone changed from sage-9.4 to sage-9.5
comment:10 Changed 9 months ago by
- Branch changed from u/chapoton/32331 to 7f4b0c0d4d422358559ee65fcc3fb46789b24a21
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
refresh sage-fixdoctests using argparse