Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clean up documentation of logic/booleval.py #8792

Closed
sagetrac-mvngu mannequin opened this issue Apr 28, 2010 · 18 comments
Closed

clean up documentation of logic/booleval.py #8792

sagetrac-mvngu mannequin opened this issue Apr 28, 2010 · 18 comments

Comments

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Apr 28, 2010

As the subject says. This needs to be coordinated with #8797.

Prerequisite: #8796

CC: @nexttime

Component: documentation

Author: Minh Van Nguyen

Reviewer: John Thurber

Merged: sage-4.6.1.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/8792

@sagetrac-mvngu sagetrac-mvngu mannequin added this to the sage-4.6.1 milestone Apr 28, 2010
@sagetrac-mvngu sagetrac-mvngu mannequin self-assigned this Apr 28, 2010
@sagetrac-mvngu

This comment has been minimized.

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented May 3, 2010

Author: Minh Van Nguyen

@sagetrac-mvngu

This comment has been minimized.

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented May 3, 2010

comment:2

Attachment: trac_8792-booleval-clean-ups.patch.gz

Changes in the patch include:

  • Add sage/logic/booleval.py to the reference manual.
  • Clean-ups in accordance with PEP 008.
  • Avoid using the name vars of a built-in function for a variable name.

@sagetrac-mvngu sagetrac-mvngu mannequin added the s: needs review label May 3, 2010
@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 19, 2010

comment:3

If I'm not mistaken, this patch does not apply against the brand new 4.4.2 with #8796 ^^;

Nathann

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented May 20, 2010

comment:4

Replying to @nathanncohen:

If I'm not mistaken, this patch does not apply against the brand new 4.4.2 with #8796 ^^;

Could you try again? Here is how I applied the relevant patches:

[mvngu@sage sage-main]$ pwd
/dev/shm/mvngu/sandbox/sage-4.4.2-8792-booleval/devel/sage-main
[mvngu@sage sage-main]$ hg tip
changeset:   14321:1451c00a8d44
tag:         tip
user:        Minh Van Nguyen <nguyenminh2@gmail.com>
date:        Wed May 19 00:55:29 2010 -0700
summary:     4.4.2

[mvngu@sage sage-main]$ hg qimport https://github.com/sagemath/sage-prod/files/10648899/trac_8796-propcalc-clean-ups.patch.gz && hg qpush 
adding trac_8796-propcalc-clean-ups.patch to series file
applying trac_8796-propcalc-clean-ups.patch
now at: trac_8796-propcalc-clean-ups.patch
[mvngu@sage sage-main]$ hg qimport https://github.com/sagemath/sage-prod/files/10648892/trac_8792-booleval-clean-ups.patch.gz && hg qpush 
adding trac_8792-booleval-clean-ups.patch to series file
applying trac_8792-booleval-clean-ups.patch
now at: trac_8792-booleval-clean-ups.patch
[mvngu@sage sage-main]$ hg tip
changeset:   14323:a91966275ff3
tag:         qtip
tag:         trac_8792-booleval-clean-ups.patch
tag:         tip
user:        Minh Van Nguyen <nguyenminh2@gmail.com>
date:        Sun May 02 20:59:37 2010 -0700
summary:     #8792: clean up documentation of logic/booleval.py

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 20, 2010

comment:5

Hmmmm... I'm really sorry but ...

~/.Sage/devel/sage-doc$ pwd
/home/ncohen/.Sage/devel/sage-doc
~/.Sage/devel/sage-doc$ hg tip
changeset:   14321:1451c00a8d44
tag:         tip
user:        Minh Van Nguyen <nguyenminh2@gmail.com>
date:        Wed May 19 00:55:29 2010 -0700
summary:     4.4.2

~/.Sage/devel/sage-doc$ hg qimport https://github.com/sagemath/sage-prod/files/10648899/trac_8796-propcalc-clean-ups.patch.gz && hg qpush
adding trac_8796-propcalc-clean-ups.patch to series file
applying trac_8796-propcalc-clean-ups.patch
now at: trac_8796-propcalc-clean-ups.patch
~/.Sage/devel/sage-doc$ hg qimport https://github.com/sagemath/sage-prod/files/10648892/trac_8792-booleval-clean-ups.patch.gz && hg qpush
adding trac_8792-booleval-clean-ups.patch to series file
applying trac_8792-booleval-clean-ups.patch
patching file sage/logic/booleval.py
Hunk #6 FAILED at 111
1 out of 6 hunks FAILED -- saving rejects to file sage/logic/booleval.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh trac_8792-booleval-clean-ups.patch
~/.Sage/devel/sage-doc$ hg tip
changeset:   14323:fd8399a20ce0
tag:         qtip
tag:         trac_8792-booleval-clean-ups.patch
tag:         tip
user:        Minh Van Nguyen <nguyenminh2@gmail.com>
date:        Sun May 02 20:59:37 2010 -0700
summary:     #8792: clean up documentation of logic/booleval.py

is there anything I am doing wrong ? O_o

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 20, 2010

comment:6

The rejects are all the fixes (a==b) => a == b at the end of your patch O_o

Nathann

@sagetrac-jthurber
Copy link
Mannequin

sagetrac-jthurber mannequin commented Nov 3, 2010

comment:8

Replying to @nathanncohen:

The rejects are all the fixes (a==b) => a == b at the end of your patch O_o

Nathann

Hi,

I'm new to development and thought this patch looked like a good place to start. I got the same error message as Nathann. Is this patch still receiving attention?

John

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 3, 2010

comment:9

Replying to @sagetrac-jthurber:

I'm new to development and thought this patch looked like a good place to start. I got the same error message as Nathann. Is this patch still receiving attention?

By commenting on the ticket, it does. ;-)

Though I personally currently have no time for it. Feel free to review it / upload a reviewer patch and we'll see...

P.S.: If the current patch doesn't apply cleanly, the ticket's status should be set to "needs work" until the patch has been rebased.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Nov 3, 2010

comment:10
$ hg import -v ~/Sage/patches/trac_8792-booleval-clean-ups.patch 
applying /home/leif/Sage/patches/trac_8792-booleval-clean-ups.patch
patching file doc/en/reference/logic.rst
patching file sage/logic/booleval.py
doc/en/reference/logic.rst
sage/logic/booleval.py

(This is with Sage 4.6. Documentation apparently builds ok, too, doctests pass.)

@williamstein
Copy link
Contributor

comment:11

It also works for me with sage-4.6.

@sagetrac-jthurber
Copy link
Mannequin

sagetrac-jthurber mannequin commented Nov 5, 2010

comment:12
07:26:25johnthurber~/sage/sage-4.6/devel/sage-test$ hg qimport ~/sage/patches/trac_8792-booleval-clean-ups-2.patch 
adding trac_8792-booleval-clean-ups-2.patch to series file
07:34:56johnthurber~/sage/sage-4.6/devel/sage-test$ hg qpush
applying trac_8792-booleval-clean-ups-2.patch
patching file sage/logic/booleval.py
Hunk #6 FAILED at 111
1 out of 6 hunks FAILED -- saving rejects to file sage/logic/booleval.py.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh trac_8792-booleval-clean-ups-2.patch

I wondered if it was my download technique, but I've tried it a couple of ways, including

hg qimport https://github.com/sagemath/sage-prod/files/10648892/trac_8792-booleval-clean-ups.patch.gz

so, unless someone has another suggestion, I will go ahead and suggest this needs to be rebased.

@williamstein
Copy link
Contributor

comment:13

Hi,

I figured it out. jthurber is using hg version 1.6, whereas Sage uses hg version 1.3. With 1.6 the algorithm for accepting hunks has been tightened (surely a good thing!) and the patch fails to apply, whereas it does apply with hg version 1.3.

@sagetrac-jthurber
Copy link
Mannequin

sagetrac-jthurber mannequin commented Nov 10, 2010

comment:14

Positive review, though there was one test timeout failure which passed when isolated from --testall.

--testall --long led to

sage -t  --long -force_lib "devel/sage/sage/modular/ssmod/ssmod.py"
*** *** Error: TIMED OUT! PROCESS KILLED! *** ***

I isolated this test, it passed:

07:36:53johnthurber~/sage/sage-4.6$ sage -t  --long -force_lib "devel/sage/sage/modular/ssmod/ssmod.py"
sage -t --long -force_lib "devel/sage/sage/modular/ssmod/ssmod.py"
	 [276.0 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 276.0 seconds
}}

@jdemeyer
Copy link

comment:15

jthurber: Please add your real name as "Reviewer" and also on Account Names mapped to Real Names

@sagetrac-jthurber
Copy link
Mannequin

sagetrac-jthurber mannequin commented Nov 11, 2010

Reviewer: John Thurber

@jdemeyer
Copy link

Merged: sage-4.6.1.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants