Fix # optional tags

[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

[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.

[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.