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 sage/misc/bitset.pyx #10093

Closed
sagetrac-mvngu mannequin opened this issue Oct 7, 2010 · 25 comments
Closed

clean up documentation of sage/misc/bitset.pyx #10093

sagetrac-mvngu mannequin opened this issue Oct 7, 2010 · 25 comments

Comments

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Oct 7, 2010

As the subject says. This ticket is part of the larger project at #7656.

Apply:

CC: @jasongrout

Component: documentation

Author: Minh Van Nguyen, Jeroen Demeyer

Reviewer: Nathann Cohen, David Coudert

Merged: sage-5.13.rc0

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

@sagetrac-mvngu sagetrac-mvngu mannequin self-assigned this Oct 7, 2010
@sagetrac-mvngu

This comment has been minimized.

@sagetrac-mvngu sagetrac-mvngu mannequin added this to the sage-4.6.1 milestone Oct 7, 2010
@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented Oct 13, 2010

comment:3

Attachment: trac-10093_cleanup-doc.patch.gz

The attached patch is a massive clean-up of the documentation and doctests in the module sage/misc/bitset.pyx. Here's a summary of what's in the patch:

  • Add the title "Bitset" as the title of the module. When generating the documentation with Sphinx, this name would become the title of the module in the generated documentation.
  • Many typo fixes and remove trailing white spaces.
  • Format code according to PEP 008 wherever possible.
  • Raise exceptions according to the style of Python 3.x.
  • Add lots more doctests to the class documentation of FrozenBitset.
  • Move input, output documentation from __init__ in FrozenBitset to the class documentation. This should make the documentation for input and output appear in the reference manual.
  • Cross reference FrozenBitset and Bitset; provide link to Python's set types in the Python library reference.
  • Explain the string representation of FrozenBitset and Bitset.
  • Get issubset, issuperset and isdisjoint to propagate exceptions; previously those methods ignored any exceptions raised.
  • Illustrate difference between length and capacity of a bitset.
  • For FrozenBitset, doctests for None type in comparison, containment, union, or, intersection, and, difference, symmetric difference, xor.
  • For Bitset, tests for None type in update, intersection_update, difference_update, symmetric_difference_update; tests for invalid input to add, remove, discard.

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented Oct 13, 2010

Author: Minh Van Nguyen

@sagetrac-mvngu

This comment has been minimized.

@jasongrout
Copy link
Member

comment:4

Note that this example: sage: FrozenBitset(()) does not contain a tuple (though it looks like it does...

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented Oct 14, 2010

Attachment: trac-10093_cleanup-doc-v2.patch.gz

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented Oct 14, 2010

comment:5

Replying to @jasongrout:

Note that this example: sage: FrozenBitset(()) does not contain a tuple (though it looks like it does...

I'm not sure what it is that you want me to modify in my patch. Do you want me to remove it? Do you want me to make some clarification in the document? The purpose of the block within which that example is found is to demonstrate different ways of constructing the empty bitset. If you want some more clarification, I have added the following examples to my patch:

  • FrozenBitet(tuple())
  • FrozenBitet(list())

See the ticket description for instruction on which patch to apply.

Also note that in the current implementation of FrozenBitset, the constructor doesn't accept the empty string. Passing the empty string to the constructor of FrozenBitset would result in a MemoryError. This is fixed in both of my patches. My intention is to say that passing the empty string to the constructor of FrozenBitset would result in the empty bitset.

@sagetrac-mvngu

This comment has been minimized.

@jasongrout
Copy link
Member

comment:6

Sorry, I should have been clearer. I read through most of the patch, and just noticed that one thing that looked confusing.

Thanks for fixing the empty string issue, and the huge number of other issues, and for formatting the docs so nicely!

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 2, 2011

comment:7

Wow, great patch ! Thank you Minh ! And thanks to whoever wrote this wonderful bitset class in the first place ! I'm making great use of it these days :-)

Just two things :

  • First, the patch does not apply on rc1, but that's just because of 2 short documentation lines
  • In the documentation the variable "iter" represents an iterable from which the FrozenBitset can be created... In Python's terminology, is an iterator an iterable ? Because in this case checking its length may be a bad idea... And converting it to a list too perhaps. Well, short of these two things, this patch is good to go (and great, being Minh's work)

Nathann

@sagetrac-mvngu
Copy link
Mannequin Author

sagetrac-mvngu mannequin commented Jan 7, 2012

comment:8

Replying to @nathanncohen:

Just two things :

  • First, the patch does not apply on rc1, but that's just because of 2 short documentation lines

This should be fixed now with attachment: trac-10093_cleanup-doc-v3.patch

  • In the documentation the variable "iter" represents an iterable from which the FrozenBitset can be created... In Python's terminology, is an iterator an iterable ? Because in this case checking its length may be a bad idea... And converting it to a list too perhaps. Well, short of these two things, this patch is good to go (and great, being Minh's work)

The variable "iter" is meant to signify that the constructor expects an iterator, something whose elements we can step through one element at a time. To do the stepping, we need an iterator: a function or something that allows us to do the stepping through from start to finish. So an iterable is different from an iterator.

@sagetrac-mvngu

This comment has been minimized.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 7, 2012

Reviewer: Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jan 7, 2012

comment:9

Well, in that case :-)

Nathann

@jdemeyer jdemeyer modified the milestones: sage-4.8, sage-5.0 Jan 9, 2012
@jdemeyer
Copy link

comment:11

This fails on 32-bit i386 Linux:

$ ./sage -t devel/sage/sage/misc/bitset.pyx
sage -t  "devel/sage/sage/misc/bitset.pyx"
**********************************************************************
File "/home/jdemeyer/sage-5.0.beta0/devel/sage/sage/misc/bitset.pyx", line 238:
    sage: FrozenBitset("", capacity=0)
Expected:
    Traceback (most recent call last):
    ...
    MemoryError...
Got:
    <BLANKLINE>
**********************************************************************
File "/home/jdemeyer/sage-5.0.beta0/devel/sage/sage/misc/bitset.pyx", line 245:
    sage: FrozenBitset("110110", capacity=0)
Expected:
    Traceback (most recent call last):
    ...
    MemoryError...
Got:
    Traceback (most recent call last):
      File "/home/jdemeyer/sage-5.0.beta0/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/jdemeyer/sage-5.0.beta0/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/jdemeyer/sage-5.0.beta0/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_1[46]>", line 1, in <module>
        FrozenBitset("110110", capacity=Integer(0))###line 245:
    sage: FrozenBitset("110110", capacity=0)
      File "bitset.pyx", line 365, in sage.misc.bitset.FrozenBitset.__init__ (sage/misc/bitset.c:4396)
    ValueError: bitset capacity does not match passed string
**********************************************************************
File "/home/jdemeyer/sage-5.0.beta0/devel/sage/sage/misc/bitset.pyx", line 249:
    sage: FrozenBitset("110110", capacity=-0)
Expected:
    Traceback (most recent call last):
    ...
    MemoryError...
Got:
    Traceback (most recent call last):
      File "/home/jdemeyer/sage-5.0.beta0/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/jdemeyer/sage-5.0.beta0/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/jdemeyer/sage-5.0.beta0/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_1[47]>", line 1, in <module>
        FrozenBitset("110110", capacity=-Integer(0))###line 249:
    sage: FrozenBitset("110110", capacity=-0)
      File "bitset.pyx", line 365, in sage.misc.bitset.FrozenBitset.__init__ (sage/misc/bitset.c:4396)
    ValueError: bitset capacity does not match passed string
**********************************************************************
1 items had failures:
   3 of  50 in __main__.example_1
***Test Failed*** 3 failures.
For whitespace errors, see the file /home/jdemeyer/.sage//tmp/bitset_11221.py
         [3.7 s]

----------------------------------------------------------------------
The following tests failed:


        sage -t  "devel/sage/sage/misc/bitset.pyx"
Total time for all tests: 3.7 seconds

@jdemeyer
Copy link

Work Issues: 32-bit

@jdemeyer
Copy link

comment:12

The same error on OS X 10.4 PPC 32-bit.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@jdemeyer
Copy link

comment:14

Attachment: trac-10093_cleanup-doc-v3.patch.gz

Rebased the original patch (which still has positive_review I guess) and added an additional patch to deal with the empty bitset.

@jdemeyer
Copy link

Changed work issues from 32-bit to none

@jdemeyer
Copy link

Changed author from Minh Van Nguyen to Minh Van Nguyen, Jeroen Demeyer

@jdemeyer

This comment has been minimized.

@dcoudert
Copy link
Contributor

comment:15

This patch passes all tests on my computer. So I change the status to positive.

@dcoudert
Copy link
Contributor

Changed reviewer from Nathann Cohen to Nathann Cohen, David Coudert

@jdemeyer
Copy link

comment:16

Attachment: 10093_empty.patch.gz

@jdemeyer
Copy link

Merged: sage-5.13.rc0

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

3 participants