Opened 9 years ago

Last modified 9 years ago

#14184 closed defect

Fix # optional tags — at Version 9

Reported by: jdemeyer Owned by: mvngu
Priority: major Milestone: sage-5.8
Component: doctest coverage Keywords:
Cc: jpflori, dimpase Merged in:
Authors: Jeroen Demeyer Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jdemeyer)

Execute

while read file; do

if [ -h $file ]; then
    continue
fi

sed -i -f /dev/stdin $file <<'EOF'

/sage: .*#/ {
s/\(#.*\) needs optional/\1 optional -/i;
s/\(#.*\) requires optional/\1 optional -/i;
s/\(#\) requires[-:, ]*\([A-Za-z0-9_]*\)[-:, ]*optional/\1 optional - \2/i;
s/\(#.*\) and optional/\1; optional/i;
s/\(#.*\) \(random\) optional/\1 \2; optional/i;
s/\(#.*\) \(long time\) optional/\1 \2; optional/i;
s/\(#.*[^; ]\)[; ]  *optional/\1; optional/i;

s/\([#;,]\) *optional and /\1 optional; /i;

s/\([#;,]\) *optional[-:, ]* needs /\1 optional - /i;
s/\([#;,]\) *optional[-:, ]* requires /\1 optional - /i;
s/\([#;,]\) *optional[-: ]* the /\1 optional - /i;
s/\([#;,]\) *optional[-: ]* GAP packages/\1 optional - gap_packages/i;
s/\([#;,]\) *optional[-: ]* GAP package \([A-Za-z0-9']*\)/\1 optional - gap_packages (\2 package)/i;
s/\([#;,]\) *optional[-: ]* GAP \(optional \)*database[s]*/\1 optional - database_gap/i;
s/\([#;,]\) *optional[-: ]* package/\1 optional -/i;
s/\([#;,]\) *optional[-: ]* \([A-Za-z0-9_]*\) command/\1 optional - \2/i;
s/\([#;,]\) *optional[-: ]* \([A-Za-z0-9_]*\) package/\1 optional - \2/i;
s/\([#;,]\) *optional[-: ]* \([A-Za-z0-9_]*\) installed/\1 optional - \2/i;
s/\([#;,]\) *optional[-: ]* \([A-Za-z0-9_]*\) and /\1 optional - \2 /i;
s/\([#;,]\) *optional[-: ]* \([A-Za-z0-9_]*\) \(not tested\)/\1 optional - \2; \3/i;

s/\([#;,]\) *optional[-: ]* internet connection/\1 optional - internet/i;
s/\([#;,]\) *optional[-: ]* database_hohel/\1 optional - database_kohel/i;
s/\([#;,]\) *optional[-: ]* Octave/\1 optional - octave/;
s/\([#;,]\) *optional[-: ]* Axiom/\1 optional - axiom/;

s/\([#;,]\) *optional[-: ]* \(.*\)'\(.*\)'/\1 optional - \2\3/i;

T;

s/\([#;,] *optional - .*[^# ]\)[# ] *$/\1/i;
s/\([#;,] *optional\)[-: ][-: ]*$/\1/i;
}

EOF
done < <(find devel/sage/sage devel/sage/doc/[a-z][a-z] -name '*.py*' -o -name '*.rst')

# Fix specific files
sed -i 's/# optional/# optional - gap_packages (Guava package)/' devel/sage/sage/coding/guava.py

Change History (9)

comment:1 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:7 follow-up: Changed 9 years ago by kcrisman

Isn't this exactly the kind of huge intrusive patch we've been trying to avoid? ;-) That said, it would be really nice to have standardization for people to look at, so I'm not opposed to this.

Stuff to fix and/or think about doing as long as this ticket is opened.

  • The sage/coding/guava.py should have the same message as the ones in sage/coding/code_bounds.py and sage/coding/linear_code.py
  • # long time; optional - internet and in another place, reversed - I assume that won't matter, but just asking in case
  • # optional - database_hohel - I don't know of a Hohel in Sage development, but I bet the database_kohel spkg is what is meant :-)
  • Did we change the convert ones to ImageMagick, or vice versa? Anyway, that should be standardized if it wasn't already elsewhere (this feels familiar)
  • I'm not sure why something can depend on dot2tex but not graphviz, and even more weirdly, graphviz is experimental, but not dot2tex ?!?!? Though I guess it is okay...
  • sometimes octave, sometimes Octave - does capitalization matter? Same with Axiom.
  • somewhat random - is that an official designation?
  • < 1/100th of a second - well, I guess it might be! I guess we can leave that.
  • What is the time command? The one that we always have? I'm confused here.
    sage: q = qsieve(n, block=False, time=True)           # optional - time 
    
  • The sage/lfunctions/sympow.py test seems ... odd. Should it instead be long test? Note that the previous command just says optional, with no indication of why.
  • # optional - FES, random - does it matter if random is before or after? I would think this is going to look for the random spkg. Same with # optional; randomly growing.

Otherwise looks good - I did look at all of them by hand, not the script. Though obviously I am not going to try to test all of these optional tests by hand.

comment:8 in reply to: ↑ 7 Changed 9 years ago by jdemeyer

Replying to kcrisman:

Isn't this exactly the kind of huge intrusive patch we've been trying to avoid? ;-)

Not really. Huge intrusive patches are okay as long as they serve a purpose. Removing whitespace all over the place doesn't serve a purpose, this patch certainly does.

This patch is mainly motivated by #12415, which has different (more strict) parsing for # optional, leading to less false positives (but also less true positives, hence this patch).

  • The sage/coding/guava.py should have the same message as the ones in sage/coding/code_bounds.py and sage/coding/linear_code.py

OK, I'll have a look.

  • # long time; optional - internet and in another place, reversed - I assume that won't matter, but just asking in case

Doesn't matter.

  • # optional - database_hohel - I don't know of a Hohel in Sage development, but I bet the database_kohel spkg is what is meant :-)

OK, so that's a typo.

  • Did we change the convert ones to ImageMagick, or vice versa? Anyway, that should be standardized if it wasn't already elsewhere (this feels familiar)

I have no idea.

  • I'm not sure why something can depend on dot2tex but not graphviz, and even more weirdly, graphviz is experimental, but not dot2tex ?!?!? Though I guess it is okay...

I have no idea what these packages are.

  • sometimes octave, sometimes Octave - does capitalization matter? Same with Axiom.

OK, changing to all lowercase.

  • somewhat random - is that an official designation?

The doctester looks for "random", so it will be recognized. I think it's fine.

  • < 1/100th of a second - well, I guess it might be! I guess we can leave that.

This can be seen as "just a comment" without further meaning.

  • What is the time command? The one that we always have? I'm confused here.

I have no idea.

  • The sage/lfunctions/sympow.py test seems ... odd. Should it instead be long test? Note that the previous command just says optional, with no indication of why.

True, it's odd. I think it means that you need to run sympow first to create some tables, but I don't know more.

  • # optional - FES, random - does it matter if random is before or after? I would think this is going to look for the random spkg. Same with # optional; randomly growing.

At least with the new doctester (which is what we're aiming for), optional - FES, random will only require FES.

Though obviously I am not going to try to test all of these optional tests by hand.

That is also most certainly not the point of this ticket.

comment:9 Changed 9 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.