Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#15096 closed defect (wontfix)

corrupted documentation (global fix)

Reported by: zimmerma Owned by:
Priority: trivial Milestone: sage-duplicate/invalid/wontfix
Component: documentation Keywords:
Cc: Merged in:
Authors: Reviewers: Paul Zimmermann, Minh Van Nguyen
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mvngu)

In Sage 5.11:

sage: from sage.structure.proof.proof import get_flag
sage: get_flag?
Type:       function
String Form:<function get_flag at 0x17e0398>
File:       /home/zimmerma/Downloads/sage-5.11/local/lib/python2.7/site-packages/sage/structure/proof/proof.py
Definition: get_flag(t=None, subsystem=None)
Docstring:
   Used for easily determining the correct proof flag to use.

   EXAMPLES:
      sage: from sage.structure.proof.proof import get_flag sage:
      get_flag(False) False sage: get_flag(True) True sage: get_flag()
      True sage: proof.all(False) sage: get_flag() False

Clearly the example part is corrupted.

This ticket aims to fix globally similar problems in the whole library.

Apply: trac_15096_2.patch

Attachments (3)

trac_15096.patch (145.8 KB) - added by zimmerma 8 years ago.
trac_15096_2.patch (126.9 KB) - added by mvngu 7 years ago.
trac_15096-regexp-docstring.patch (1.2 KB) - added by jhpalmieri 7 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 8 years ago by zimmerma

  • Description modified (diff)
  • Summary changed from corrupted documentation of get_flag to corrupted documentation (global fix)

comment:2 Changed 8 years ago by zimmerma

  • Description modified (diff)

Changed 8 years ago by zimmerma

comment:3 Changed 8 years ago by zimmerma

  • Authors set to Paul Zimmermann
  • Status changed from new to needs_review

in the attached patch, I've replaced everywhere:

   EXAMPLES:
      sage:

by

   EXAMPLES::

      sage:

Paul

comment:4 Changed 8 years ago by chapoton

This is called a "patchbomb", and is not somethingone really wants to do. Patches are better when they are smaller and only touch a few files..

comment:5 Changed 8 years ago by zimmerma

  • Description modified (diff)

comment:6 Changed 7 years ago by mvngu

With Sage 5.13.beta0, I'm getting a bunch of failures with the patch:

applying trac_15096.patch
patching file sage/logic/boolformula.py
Hunk #1 FAILED at 9
Hunk #2 FAILED at 151
Hunk #3 FAILED at 175
Hunk #4 FAILED at 192
Hunk #5 FAILED at 217
Hunk #6 FAILED at 242
Hunk #7 FAILED at 263
Hunk #8 FAILED at 284
Hunk #9 FAILED at 305
Hunk #10 FAILED at 326
Hunk #11 FAILED at 348
Hunk #12 FAILED at 369
Hunk #13 FAILED at 391
Hunk #14 FAILED at 428
Hunk #15 FAILED at 500
Hunk #16 FAILED at 521
Hunk #17 FAILED at 547
Hunk #18 FAILED at 573
Hunk #19 FAILED at 600
Hunk #20 FAILED at 685
Hunk #21 FAILED at 715
Hunk #22 FAILED at 775
Hunk #23 FAILED at 844
Hunk #24 FAILED at 881
Hunk #25 FAILED at 910
Hunk #26 FAILED at 955
Hunk #27 FAILED at 989
Hunk #28 FAILED at 1022
Hunk #29 FAILED at 1051
Hunk #30 FAILED at 1074
Hunk #31 FAILED at 1116
31 out of 31 hunks FAILED -- saving rejects to file sage/logic/boolformula.py.rej
patching file sage/logic/logic.py
Hunk #1 FAILED at 32
Hunk #2 FAILED at 66
Hunk #3 FAILED at 122
Hunk #4 FAILED at 177
4 out of 4 hunks FAILED -- saving rejects to file sage/logic/logic.py.rej
patching file sage/logic/logicparser.py
Hunk #1 FAILED at 4
Hunk #2 FAILED at 28
Hunk #3 FAILED at 52
Hunk #4 FAILED at 118
Hunk #5 FAILED at 149
Hunk #6 FAILED at 193
6 out of 6 hunks FAILED -- saving rejects to file sage/logic/logicparser.py.rej
patching file sage/misc/sage_input.py
Hunk #24 succeeded at 1707 with fuzz 2 (offset 465 lines).
Hunk #25 succeeded at 1758 with fuzz 2 (offset 489 lines).
Hunk #26 succeeded at 1777 with fuzz 2 (offset 491 lines).
Hunk #27 succeeded at 1794 with fuzz 2 (offset 489 lines).
Hunk #28 succeeded at 1883 with fuzz 2 (offset 537 lines).
Hunk #29 succeeded at 1983 with fuzz 2 (offset 616 lines).
Hunk #30 succeeded at 2069 with fuzz 2 (offset 677 lines).
Hunk #31 succeeded at 2110 with fuzz 2 (offset 701 lines).
Hunk #32 succeeded at 2128 with fuzz 2 (offset 697 lines).
Hunk #33 succeeded at 2147 with fuzz 2 (offset 697 lines).
Hunk #34 succeeded at 2213 with fuzz 2 (offset 746 lines).
Hunk #35 succeeded at 2230 with fuzz 2 (offset 748 lines).
Hunk #36 succeeded at 2247 with fuzz 2 (offset 750 lines).
Hunk #37 succeeded at 2300 with fuzz 2 (offset 788 lines).
Hunk #38 succeeded at 2317 with fuzz 2 (offset 790 lines).
Hunk #39 succeeded at 2332 with fuzz 2 (offset 788 lines).
Hunk #40 succeeded at 2441 with fuzz 2 (offset 878 lines).
Hunk #41 succeeded at 2457 with fuzz 2 (offset 879 lines).
Hunk #42 succeeded at 2472 with fuzz 2 (offset 879 lines).
Hunk #43 succeeded at 2605 with fuzz 2 (offset 995 lines).
Hunk #44 succeeded at 1632 with fuzz 1 (offset -3 lines).
Hunk #45 succeeded at 2163 with fuzz 2 (offset 491 lines).
Hunk #47 succeeded at 1835 with fuzz 2 (offset 122 lines).
Hunk #49 succeeded at 1852 with fuzz 2 (offset 87 lines).
Hunk #50 succeeded at 1869 with fuzz 2 (offset 85 lines).
Hunk #51 succeeded at 2269 with fuzz 2 (offset 468 lines).
Hunk #53 succeeded at 1933 with fuzz 2 (offset 91 lines).
Hunk #54 succeeded at 1953 with fuzz 2 (offset 95 lines).
Hunk #55 succeeded at 1973 with fuzz 2 (offset 98 lines).
Hunk #56 succeeded at 2359 with fuzz 2 (offset 465 lines).
Hunk #58 succeeded at 2034 with fuzz 2 (offset 95 lines).
Hunk #59 succeeded at 2050 with fuzz 2 (offset 91 lines).
Hunk #61 succeeded at 2505 with fuzz 2 (offset 507 lines).
Hunk #63 succeeded at 2646 with fuzz 2 (offset 606 lines).
Hunk #64 succeeded at 2665 with fuzz 2 (offset 609 lines).
Hunk #65 succeeded at 2832 with fuzz 2 (offset 760 lines).
Hunk #66 succeeded at 2876 with fuzz 2 (offset 788 lines).
Hunk #68 succeeded at 2849 with fuzz 2 (offset 719 lines).
Hunk #69 succeeded at 2863 with fuzz 2 (offset 715 lines).
Hunk #70 succeeded at 2978 with fuzz 2 (offset 811 lines).
Hunk #71 succeeded at 3031 with fuzz 2 (offset 848 lines).
Hunk #73 succeeded at 3002 with fuzz 2 (offset 767 lines).
Hunk #74 succeeded at 3019 with fuzz 2 (offset 767 lines).
Hunk #75 succeeded at 3104 with fuzz 2 (offset 835 lines).
Hunk #76 succeeded at 3150 with fuzz 2 (offset 865 lines).
Hunk #78 succeeded at 3121 with fuzz 2 (offset 797 lines).
Hunk #79 succeeded at 3136 with fuzz 2 (offset 795 lines).
Hunk #80 FAILED at 2353
Hunk #81 succeeded at 3173 with fuzz 2 (offset 802 lines).
Hunk #83 FAILED at 2463
Hunk #84 FAILED at 2478
Hunk #85 FAILED at 2492
Hunk #86 succeeded at 2577 with fuzz 2 (offset 66 lines).
Hunk #87 succeeded at 2694 with fuzz 2 (offset 116 lines).
Hunk #89 FAILED at 2627
Hunk #90 FAILED at 2649
Hunk #91 FAILED at 2667
Hunk #92 succeeded at 2721 with fuzz 2 (offset 28 lines).
Hunk #93 succeeded at 2796 with fuzz 2 (offset 77 lines).
Hunk #94 succeeded at 2911 with fuzz 2 (offset 117 lines).
Hunk #96 FAILED at 2837
Hunk #97 FAILED at 2852
Hunk #98 FAILED at 2865
Hunk #99 succeeded at 3064 with fuzz 2 (offset 181 lines).
Hunk #100 succeeded at 3230 with fuzz 2 (offset 323 lines).
Hunk #103 FAILED at 2982
Hunk #104 FAILED at 3004
Hunk #105 FAILED at 3020
Hunk #106 FAILED at 3034
Hunk #107 FAILED at 3054
Hunk #109 FAILED at 3104
Hunk #110 FAILED at 3119
Hunk #111 FAILED at 3133
Hunk #112 FAILED at 3149
Hunk #113 FAILED at 3168
Hunk #115 FAILED at 3216
21 out of 122 hunks FAILED -- saving rejects to file sage/misc/sage_input.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh trac_15096.patch

I'm going to split up the patch into multiple patches, to make it easier to review and merge.

Changed 7 years ago by mvngu

comment:7 Changed 7 years ago by mvngu

  • Description modified (diff)
  • Reviewers set to Minh Van Nguyen

OK, so I didn't split Paul's patch into multiple patches. It turns out that you don't need to include patches for files under the sage/logic directory. Someone just needs to review the updated patch trac_15096_2.patch​.

comment:8 Changed 7 years ago by jdemeyer

  • Authors Paul Zimmermann deleted
  • Milestone changed from sage-5.13 to sage-duplicate/invalid/wontfix
  • Reviewers changed from Minh Van Nguyen to Paul Zimmermann, Minh Van Nguyen
  • Status changed from needs_review to positive_review

Sorry, but we cannot allow such a patch. While I agree that the changes it makes are desirable, it will lead to massive merge conflicts (for the same reason, we cannot simply strip all trailing white-space, even though we would like to).

comment:9 Changed 7 years ago by jdemeyer

  • Resolution set to wontfix
  • Status changed from positive_review to closed

comment:10 follow-up: Changed 7 years ago by vbraun

It would still be nice to fix this. How about

  • Write a lint-like script that checks that the docstrings adhere to the Sage style
  • Put it into the git commit hook, so whenever you change a file you'll be nagged to fix the docstrings in there.
  • Programmatically fix files that haven't been changed in a year, say.

Some parts of sage are under development, some have stabilized a long time ago. By just changing the stable parts we'd avoid most conflicts.

On another ticket, of course ;-)

comment:11 in reply to: ↑ 10 Changed 7 years ago by jdemeyer

Replying to vbraun:

  • Programmatically fix files that haven't been changed in a year, say.

Some parts of sage are under development, some have stabilized a long time ago. By just changing the stable parts we'd avoid most conflicts.

I don't agree with your assumption that code which hasn't changed in a year is "stable".

comment:12 Changed 7 years ago by vbraun

I meant "stable" as in not changing. It may crash and burn, of course.

comment:13 Changed 7 years ago by jhpalmieri

I think that the issue is not, for the most part, people producing incorrectly formatted docstrings, but rather docstrings dating from the pre-Sphinx days which used the old style of

EXAMPLE:
    sage: ...

So maybe a better option is to change the sphinxify function (which I think is responsible for producing these formatted docstrings) to replace "EXAMPLE(S?):\n text" with "EXAMPLES::\n\n text". That change should be an easy regexp operation and should fix most of these problems.

(Of course, if it's not sphinxify, then fix the relevant function...)

comment:14 follow-up: Changed 7 years ago by vbraun

I think that'll just cause more confusion, now at least you'll find out about the correct syntax if you look at the output.

comment:15 in reply to: ↑ 14 Changed 7 years ago by jhpalmieri

Replying to vbraun:

I think that'll just cause more confusion, now at least you'll find out about the correct syntax if you look at the output.

This makes sense from a developer's point of view, but the current state of affairs could be very confusing to users. Fixing it, at the expense of confusing developers, seems like a worthwhile trade.

Here's a patch. We could add a warning ("Warning: old-style docstring detected") or something like that.

comment:16 Changed 7 years ago by jhpalmieri

By the way, this patch is not ready to go; it should test that the indentation level increases after the "EXAMPLES" line: it shouldn't take effect in situations like

EXAMPLES:

This illustrates something::

    sage: ...

Changed 7 years ago by jhpalmieri

comment:17 Changed 7 years ago by jhpalmieri

This version should work better.

Note: See TracTickets for help on using tickets.