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

Parent/Element relation for ChainComplex/Chain #15161

Closed
vbraun opened this issue Sep 5, 2013 · 34 comments
Closed

Parent/Element relation for ChainComplex/Chain #15161

vbraun opened this issue Sep 5, 2013 · 34 comments

Comments

@vbraun
Copy link
Member

vbraun commented Sep 5, 2013

Nothing in sage.homology makes use of the Parent/Element framework. But it would be nice to be able to work with chains and use normal syntax for addition and multiplication by scalars. The aim of this ticket is to adapt chain complexes. The analogous project for cell complexes will be left for a future ticket.

Next to code cleanup, I also added ascii art support:

sage: C = ChainComplex([matrix(QQ, 3, 1), matrix(ZZ, 4, 3)])
sage: C
Chain complex with at most 3 nonzero terms over Rational Field
sage: %display ascii_art
sage: C
            [0 0 0]                
            [0 0 0]       [0]      
            [0 0 0]       [0]      
            [0 0 0]       [0]      
 0 <-- C_2 <-------- C_1 <---- C_0 <-- 0 
sage: C.an_element()
   d_2  [  9]  d_1  [-1/2]  d_0        d_-1  
0 <---- [ -1] <---- [   1] <---- [-1] <----- 0
        [1/5]       [   1]           
        [2/7]                        

CC: @jhpalmieri @tscrim

Component: algebraic topology

Author: Volker Braun

Branch/Commit: public/homology/chain_complex_parents-15161 @ e955feb

Reviewer: John Palmieri, Travis Scrimshaw

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

@vbraun vbraun added this to the sage-5.13 milestone Sep 5, 2013
@vbraun
Copy link
Member Author

vbraun commented Sep 5, 2013

Author: Volker Braun

@vbraun
Copy link
Member Author

vbraun commented Sep 5, 2013

Branch: u/vbraun/chain_complex_parents

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 5, 2013

Commit: d0dde81

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 5, 2013

Branch pushed to git repo; I updated commit sha1. New commits:

[changeset:d0dde81]Refactoring ChainComplex as a Parent for Chains
[changeset:d8713eb]Merge remote-tracking branch 'origin/build_system' into public/sage-git/master
[changeset:970090d]Merge branch 'u/ohanar/build_system'
[changeset:cf14c84]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:2f871b4]make merge() accept a ticket number
[changeset:0777127]fixed upload_ssh_key(): do not bother the user with this question every time
[changeset:060184c]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:272516e]fixed doctests
[changeset:992a5a4]fix ssh key generation in dev scripts
[changeset:251893d]fixed gather()
[changeset:2b20053]display all branches in the same way
[changeset:b95a820]Merge branch 'ticket/14482' into public/sage-git/master
[changeset:edc0d30]fix editing tickets that contain lines ending in colons
[changeset:1f82abc]Better handling of empty branch fields.
[changeset:a85aa32]better handling of empty branch field.
[changeset:7b46734]fixed a typo.
[changeset:a10bc4f]Merge branch 'ticket/14482'
[changeset:59994e6]use SageDevWrapper in ./sage --dev
[changeset:10f0669]None is not a valid ticket name
[changeset:f37c12d]fixed unstash help string
[changeset:bc4db82]Fix interactive commit.
[changeset:fda55a9]Merge branch 'u/saraedum/ticket/14482' of ssh://trac.sagemath.org:2222/sage into HEAD
[changeset:22d879a]allow modification of status
[changeset:3566819]fixed doctest
[changeset:7aed8e1]fixed time zone in _rewrite_patch_header
[changeset:e51bbec]docstring for _rewrite_patch and _detect_patch_modified_files
[changeset:58445e7]implemented upload_ssh_key
[changeset:bd6c846]fixed bug in _is_ticket_name
[changeset:5c1f210]implemented show_dependencies
[changeset:92fdbd1]add `_or_` hack to help strings
[changeset:d92a271]implemented diff()
[changeset:eca9462]do not accept ticket numbers as branch names
[changeset:54b58e8]doctests for vanilla()
[changeset:36042d8]implemented gather() and merge()
[changeset:b26db73]doctest for abandon()
[changeset:042dcda]fixed download()
[changeset:16412e2]doctest cleanup
[changeset:07bdebc]fixed download()
[changeset:a4ccf4b]expose abandon()
[changeset:14336b7]syntactic sugar for arguments containing _or_
[changeset:8d0bfc5]implemented abandon() and vanilla()
[changeset:44a5782]implemented local_tickets()
[changeset:7e9dad0]fixed printing in cmd_line_interface
[changeset:8f25bd0]simplified doctest setup
[changeset:20b9214]added TODOs
[changeset:1f04633]fixed doctests
[changeset:35d311b]fixed doctests
[changeset:60d85d5]syntactic sugar for git_interface
[changeset:e1f40c1]fixed doctests
[changeset:a07dbfc]removed unused file
[changeset:6d3293f]use urlretrieve instead of wget
[changeset:2e03443]fixed nop detection in upload()
[changeset:0411f15]doctests for download_patch
[changeset:9c1f55b]delete downloaded patch file in import_patch
[changeset:78e70b5]fix initial master branch for trac server
[changeset:8f3e052]fixed relative paths for untracked-files
[changeset:40b015f]fixed a problem when uploading dependencies
[changeset:3f6d35d]The default master branch is "master".
[changeset:6b0755c]made sage --dev usable again
[changeset:3522a34]import SUPER_SILENT globally in sagedev.py
[changeset:00f7f5a]fixed import-patch
[changeset:9dc4f41]fixed remote_status()
[changeset:96b8266]fixed some typos
[changeset:e19bd8d]doctests for browse_ticket()
[changeset:6fd577c]doctests for add_comment
[changeset:f182d72]fixed doctest for stash
[changeset:31d1cfc]simplified doctests
[changeset:cb2f40a]doctests for edit_ticket()
[changeset:e7357d6]fixed unstash()
[changeset:9589d4f]show affected files when cleaning working directory
[changeset:a4f06b5]made select() less confusing
[changeset:270f1e3]fixed two typos in doctests
[changeset:15cf131]doctests for reset_to_clean_state()
[changeset:9e70145]completed upload()
[changeset:fa584f6]doctests for set_remote
[changeset:4b5a19e]added doctests for switch_branch
[changeset:6599f77]added doctests for create_ticket()
[changeset:254f22d]added authors
[changeset:4b48f71]doctests for test.sagedev
[changeset:07014ac]doctests for trac_error
[changeset:e332d57]fixed doctests
[changeset:d5ad059]modified git error reporting
[changeset:1b5139d]sage --dev uses the dev wrapper
[changeset:604ecf1]a very limited version of upload()
[changeset:7a11f93]implemented commit
[changeset:753e9c3]removed comments from old methods
[changeset:c2e39b7]being more specified about some of the todos
[changeset:6f9af2f]enabled patch methods
[changeset:262bc9f]unstash() stub
[changeset:e8236ac]fixed doctests
[changeset:f00d069]omit --git-dir from debug output
[changeset:bebf377]easier debugging for user_interface
[changeset:afc0a6b]fixed doctests and added TODOs
[changeset:acf6803]branches may not contain a slash
[changeset:31da75d]do not check user and email on every git action
[changeset:e65035d]easier debugging of git errors
[changeset:36db105]wrapper for SageDev()
[changeset:f6ccae3]better error reporting in SageDev
[changeset:6b0a81e]wrap sagedev for better error reporting
[changeset:8c61f06]better git error reporting
[changeset:2478348]make sure git user.name and user.email are set
[changeset:fa6717a]fixed doctests for download()
[changeset:71f4d1f]fixed some more doctests in sagedev.py
[changeset:050e3a8]more doctests for sagedev
[changeset:391f101]added ls-remote
[changeset:913e92d]fixed server_proxy
[changeset:032a603]work on doctesting SageDev()
[changeset:ee62afe]added DoctestSageDev
[changeset:7b93930]added _branch_for_ticket to trac_interface
[changeset:43f45bc]renamed branch_exists and ref_exists
[changeset:eecb29a]saving dicts for doctests
[changeset:39c357b]protect saving dict in doctests
[changeset:d82820b]convenient logging methods
[changeset:16dea3c]initialize empty repository on trac_server
[changeset:ae0a817]better trac error reporting
[changeset:f390ad6]fixes a bug in config
[changeset:eae2477]added doctests for DoctestServerProxy
[changeset:a5305dc]added doctests for DoctestTracInterface
[changeset:d877773]added doctests for trac_server.py
[changeset:9c85110]added doctests to trac_interface.py
[changeset:8e7e31c]fixed doctests in saving_dict.py
[changeset:7dd6d08]fixed doctests in user_interface.py
[changeset:0567dae]added doctests for git_error.py
[changeset:4dc4e18]fixed doctests in digest_transport
[changeset:9fc6b04]doctests for GitInterface
[changeset:121d9c4]fixed doctests for GitInterface
[changeset:ed09fe3]work on git doctests
[changeset:36f3c7f]save creation of the global SageDev object in doctests
[changeset:b5947ff]updated DoctestConfig
[changeset:0085579]improved git wrapper
[changeset:4853940]fixed GitError
[changeset:1a57439]fixed config.py doctests
[changeset:88ec561]configurable log level
[changeset:a752a82]fixed doctests in CmdLineInterface
[changeset:9f86e98]added fixed doctests
[changeset:2601875]fixed docstrings
[changeset:c79f3c6]docstring cleanup
[changeset:930624e]split up files
[changeset:210f868]Merge remote-tracking branch 'refs/remotes/trac/u/tkluck/ticket/14482' into ticket/14482
[changeset:ecb4ca7]when not passing a ticket number, let download-patch take the current one
[changeset:30e71c8]Add a browse-ticket command to the development scripts
[changeset:e29fd9f]some initial work on doctests
[changeset:894a428]added add-comment
[changeset:dd04fa3]added edit_ticket
[changeset:b5ae594]fixed some syntax errors
[changeset:fee1ddb]warn about dependency changes
[changeset:cc43b27]git commands raise an exception if their exit code is non-zero
[changeset:751d40a]Replace print statement by call to _UI.show in developement scripts.
[changeset:4a07e61]Update dependencies correctly, when downloading via developement scripts.
[changeset:e98e71e]Change condition when dev._UI displays a message using the pager.
[changeset:77c959d]Merge remote-tracking branch 'refs/remotes/trac/public/sage-git/ticket/14482' into ticket/14482
[changeset:61236da]Merge remote-tracking branch 'trac/public/sage-git/ticket/14482'
[changeset:cad2b35]Merge branch 'u/saraedum/ticket/14482' of ssh://trac.sagemath.org:2222/sage into ticket/14482
[changeset:17b9944]fixed a problem with non-string arguments to git
[changeset:308e556]Merge remote-tracking branch 'refs/remotes/trac/u/saraedum/ticket/14482' into ticket/14482
[changeset:b74c4b6]Merge remote-tracking branch 'refs/remotes/trac/u/mraum/ticket/14482' into ticket/14482
[changeset:08d3e94]Merge branch 'ticket/14482'
[changeset:e5dc9c6]Use public/sage-git/master as default base
[changeset:67e8c8e]Merge remote-tracking branch 'trac/u/mraum/ticket/14482' into ticket/14482
[changeset:0f48ce0]DOCTEST_MODE must not be imported since it may change when running doctests from within sage
[changeset:70d1918]More DOCTEST_MODE fixes.
[changeset:4543889]doctest mode fix
[changeset:1446290]Make documentation strings in sagedev.py raw.
[changeset:3f8e37e]Update the parser used in sage-dev.
[changeset:063d879]When parsing arguments for sage --dev, do not evaluate parameters.
[changeset:c8e7802]Add detailed error message at the end of _remote_pull_branch.
[changeset:a1fd9b5]In remote_status, handle ticket==None.
[changeset:50e1ece]Merge branch 'ticket/14482'
[changeset:2e2a973]Merge remote-tracking branch 'trac/u/saraedum/ticket/14482' into ticket/14482
[changeset:7477173]for now, use u/tkluck/master as the base for new tickets
[changeset:a9878a5]Merge remote-tracking branch 'refs/remotes/trac/u/tkluck/ticket/14482' into ticket/14482
[changeset:f41fd6e]ref hack for merge
[changeset:ebdbf6e]fixes to merge when merging non-tickets
[changeset:1ca56cf]Merge branch 'ticket/14273' into tip
[changeset:aa60895]remove workaround for sagenb pull request 84 (#14273)
[changeset:133785b]Merge branch 'ticket/14482' into tip
[changeset:3614ef2]Merge branch 'ticket/14330' into tip
[changeset:ceff7a6]fixed branch merge
[changeset:70bf95d]fixed upload() to look at the trac ticket
[changeset:5423f37]fixed a typo
[changeset:0c4196b]fixed download_patch
[changeset:93cabf9]fixed import-patch to work again with urls
[changeset:bb9f7ac]Upgrade sagenb to version 0.10.7.1
[changeset:fa27b05]Add sage --git commandline
[changeset:fc106d1]Fixup for remote-status
[changeset:4424390]temporary fix for the fact that the trac server uses an invalid certificate
[changeset:fd1b63b]Obtain trac server name from environment
[changeset:ab95861]Small fixes for sage --dev remote-status
[changeset:e8761c6]Fix interpretation of return value for mkstemp
[changeset:79e2959]fixed creation in switch_ticket
[changeset:3809fb6]fixed dependency recording in merge()
[changeset:8fd3f4b]removed some obsolete (?) _print calls
[changeset:30aa722]fixed upload of dependencies
[changeset:b37f226]echo git commands
[changeset:795a35b]fixed show_dependencies
[changeset:f559e50]do not ignore return value of push
[changeset:8b3b995]added missing default repository
[changeset:4d2eff7]removed references to git._dependencies
[changeset:7fa90b1]removed unused _fetch() method that is almost identical with sagedev._fetch
[changeset:a61bfd7]split SAGE_REPO variable into SAGE_REPO_ANONYMOUS and SAGE_REPO_AUTHENTICATED
[changeset:6783072]overwrite the merge_head when pulling
[changeset:4d5e459]fixes an undefined name
[changeset:6dc083d]remove references to git._ticket
[changeset:be97a15]add --patch appears to be a friendlier interactive mode than add --interactive
[changeset:66cbbe0]master should be build_system until we can work off master
[changeset:ec50d33]Added missing import
[changeset:37ccf07]fixed a typo
[changeset:b9cfbea]fixed some sage-dev startup problems
[changeset:0c7c241]Merge branch 'u/ohanar/dev_scripts' of ssh://trac.sagemath.org:2222/sage into dev_scripts
[changeset:3877249]dev_scripts: add pre-commit for auto-striping trailing whitespace
[changeset:3a319d0]dev_scripts: first part of major cleanup/reorg of SageDev
[changeset:dae0ade]Merge branch 'build_system' into dev_scripts
[changeset:75eb0c7]sagedev: added format-patch to git interface
[changeset:38dfeba]sagedev: rework GitInterface._run_git to use Popen
[changeset:ef3c4e5]sagedev: don't use shell=True in subprocesses
[changeset:2991053]sage-dev: a little more rewriting of TracInterface
[changeset:f0adb30]sage-dev: cleanup dependencies method
[changeset:356be84]sage-dev: add url to output for create_ticket_interactive -> create_ticket
[changeset:9f42aa9]sage-dev: add methods for setting trac username/password
[changeset:ac756d2]sage-dev: smarter command line show method
[changeset:2da44d4]sage-dev: optional flag online -> internet
[changeset:b66a1fb]sage-dev: document and doctest DoctestServerProxy
[changeset:589a47d]sage-dev: fix typo
[changeset:a4b93c9]sage-dev: add trac login checking
[changeset:b94112a]sage-dev: use DOCTEST_MODE in git_interface
[changeset:33eeaa6] sage-dev: fixup some online tests
[changeset:3ae819c]sage-dev: fixup edit_ticket
[changeset:bd3eb58]sage-dev: fix password doctests
[changeset:3b9e6ba]sage-dev: redo auth timeout so as to not keep password as an attr
[changeset:f03817c]sage-dev: ticketnum must always be an int
[changeset:c844123]sage-dev: inherit from SafeTransport
[changeset:2ea0e7c]sage-dev: prefer anonymous proxy
[changeset:f10e619]sage-dev: fix authenticated substitution in anonymous proxy
[changeset:210d8af]sage-dev: cleanup request -> single_request
[changeset:052a6bc]sage-dev: better password timeout
[changeset:4a186de]sage-dev: small cleanup of Config
[changeset:ff4b8af]sage-dev: more pythonic TracInterface._password
[changeset:df8a352]sage-dev: cleanup proxy servers
[changeset:cb6e349]sage-dev: add some documentation/doctests
[changeset:a9ae96d]sage-dev: cleanup create_ticket and improve ticket parsing
[changeset:4bfab7c]sage-dev: make password timeout relative to last use
[changeset:ca20a52]sage-dev: password timeout should start after all prompts
[changeset:921739c]sage-dev: cleanup some doctests
[changeset:fe25bc9]sage-dev: rework trac password
[changeset:cd7a7d2]sage-dev: have Config auto-write to disk
[changeset:aef4bfc]sage-dev: refactor UserInterface
[changeset:d02588c]sage-dev: cleanup and add some doctests to ticket editing
[changeset:074f42b]sage-dev: add edit method to UserInterface
[changeset:c95a125]sage-dev: fix a lot of doctests in trac_interface
[changeset:09d279a]sage-dev: finish up doctests for user interface
[changeset:d44599e]sage-dev: add a bunch of tests
[changeset:9f019b0]sage-dev: docstring for commit_all should be literal
[changeset:5ef9178]sage-dev: make `__call__` alias for execute
[changeset:b7ba479]sage-dev: improve doc for git.execute
[changeset:9195359]sage-dev: document and do a bit of cleanup with _run_git
[changeset:c91fcea]sage-dev: make UI more library friendly
[changeset:e3fb66e]sage-dev: fix broken toc in docbuilding
[changeset:65561b5]sage-dev: remove authenticated object
[changeset:39fe215]sage-dev: add doctests for patches with renames
[changeset:4f57c7a]sage-dev: support rename directives in patches
[changeset:a9c8e0c]sage-dev: closer to finishing GitInterface
[changeset:a7abc5b]sage-dev: yet more fleshing out of GitInterface
[changeset:823252f]sage-dev: fix a couple of derps
[changeset:167018a]sage-dev: even more fleshing out of GitInterface
[changeset:43f0b68]sage-dev: more doctests
[changeset:0ac2c65]sage-dev: better tmp cleanup
[changeset:fad8754]sage-dev: spoofing commit dates is faster
[changeset:bdf50ba]sage-dev: don't catch CalledProcessErrors
[changeset:5b9bac9]sage-dev: create temporary directory only when necessary
[changeset:e674d25]sage-dev: use new doctest continuation format
[changeset:72b2f33]sage-dev: sage_dev -> dev
[changeset:2b7c7be]sage-dev: fix doctest for _raise
[changeset:266ab3a]sage-dev: more fleshing out of GitInterface
[changeset:6ed2139]sage-dev: always define doctest_config
[changeset:f9a145c]sage-dev: some documentation fixes
[changeset:a355ca1]sage-dev: more fleshing out of GitInterface
[changeset:65f192e]sage-dev: remove stupid logic
[changeset:f169308]sage-dev: fix tmp_dir
[changeset:294481b]sage-dev: finish SavingDict
[changeset:e143d43]sage-dev: add to docbuilding
[changeset:3a0bdda]sage-dev: require default argument to be callable
[changeset:e107a03]sage-dev: rewrite SavingDict
[changeset:8edc13f]sage-dev: overhaul branch conversions again, add examples/tests
[changeset:1716df7]sage-dev: overhaul branch conversions
[changeset:101d51b]sage-dev: more doctests for GitInterface
[changeset:c1f930d]sage-dev: use DOCTEST_MODE to ease doctesting
[changeset:46018cd]sage-dev: simplify GitInterface.has_uncommitted_changes
[changeset:8f7a067]sage-dev: work towards making the git interface doctestable
[changeset:d5825b1]sage-dev: make Config inherit from collections.MutableMapping
[changeset:8f29198]sage-dev: fix a lot of doctests
[changeset:fc333ed]sagedev: overhaul command line parsing
[changeset:ac4bd15]sage-dev: fix path for git repository
[changeset:67c4e30]sage-dev: don't import scripts if SAGE_ROOT is not defined
[changeset:b7fab4d]sage-dev: integrate scripts from sage-workflow

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2013

Changed commit from d0dde81 to 7e075ed

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 7, 2013

Branch pushed to git repo; I updated commit sha1. New commits:

[changeset:7e075ed]Fixed super categories and all doctests
[changeset:7543b85]finished refactoring the homology() method
[changeset:ed95223]Chains (elements of chain complexes) and their module structure
[changeset:0a6eabe]Ascii art and more code refactoring for chain complexes

@vbraun
Copy link
Member Author

vbraun commented Sep 7, 2013

comment:4

comment:2 is a minor screwup of the scripts, only the first commit should have been listed...

In any case, all tests pass for me and I did what I wanted to do on this ticket. Ready for review...

@vbraun

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:5

I don't understand the example C.an_element() in the ticket description: all of the differentials are zero, so the various listed elements can't be mapping to each other. I would expect that an element would be an element of a single C^i, or possibly an element of the direct sum of all of the C^i's, and I would prefer the first choice.

@vbraun
Copy link
Member Author

vbraun commented Sep 8, 2013

comment:6

Here, element = chain, i.e. an element of the direct sum of the C_i. That is the most general notion of element, so it is the one that ought to be implemented. The arrows in the ascii art are there to visualize the relative position in the complex. And they are \to and not \mapsto arrows... Do you have a better ascii art representation in mind?

Since it is useful to see the coordinate expression of a chain that is supported only in a single degree (e.g. homology generators) they are printed differently:

sage: D = ChainComplex({0: matrix(ZZ, 2, 2, [1,0,0,2])})
sage: D()
Trivial chain
sage: D({0:[2, 3]})
Chain(0:(2, 3))
sage: D({0:[2, 3], 1:[4, 5]})
Chain with 2 nonzero terms over Integer Ring
sage: z = D({1:[0,1]})
sage: z.is_boundary()
False
sage: (2*z).is_boundary()
True

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2013

Branch pushed to git repo; I updated commit sha1. New commits:

[changeset:8ec7ded]fixed bug in is_boundary()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 8, 2013

Changed commit from 7e075ed to 8ec7ded

@jhpalmieri
Copy link
Member

comment:8

I've been busy with various departmental duties for the past month, but I'd like to start reviewing this soon. I don't know how to review git tickets, though, and the directions I've found seem to be contradictory (if I download Sage from git and type make, it fails because the top-level Makefile refers to spkg/pipestatus, while everything has been moved from spkg to build. If I switch branches to build_system, then it builds but ./sage --dev ... doesn't work).

Furthermore, I don't know how to look at the relevant changes easily. Do I really need to look at half a dozen different changesets, or is there a unified view somewhere? On this ticket, the "branch" field doesn't seem to be active, but on a ticket like #15202, if I click on "Commits" I get a pretty diagram, and then from there if I click on "View changes", I get "504 Gateway Time-out. The server didn't respond in time." I guess I'm supposed to click on the branch name instead?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2013

Branch pushed to git repo; I updated commit sha1. New commits:

[changeset:b2710cd]Merge branch 'master' into chain_complex_parents

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 7, 2013

Changed commit from 8ec7ded to b2710cd

@vbraun
Copy link
Member Author

vbraun commented Oct 7, 2013

comment:10

Starting this week, using git should be relatively simple. If you download the current master from github (say) then installation should just work. See http://trac.sagemath.org/wiki/QuickStartSageGit

The developer scripts should also work as in http://sagemath.github.io/git-developer-guide/walk_through.html:

sage -dev checkout --ticket 15161

gets you the source code. Then make/test as usual. To see all source code changes relative to the master branch, use

sage -dev diff --base master
  • I just merged in the current master so this ticket contains the dev scripts, too.

  • The changeset browser in trac still needs some love...

@jhpalmieri
Copy link
Member

comment:11

On a quick reading of chain_complex.py:

  • Do you want to add your name to the AUTHORS list at the top of chain_complex.py?
  • In the docstring for ChainComplex_class, change `base_ring` to ``base_ring``
  • In the docstring for differential, the spaces aren't lined up right in the indented part for dim. Same thing happens several other places. I guess I ought to be able to fix issues like this and then push them to a branch of my own. Meanwhile, I've attached a patch file with the changes.
  • In betti, well, first, I hope we are moving toward using CHomP as much as possible for homology computations. Given this, if we have a chain complex over a finite field, it may be faster to CHomP to compute its homology (hence its Betti numbers) than to use Sage to compute its homology over Q (which is a little strange anyway, if the chain complex is over a finite field). So the beginning part of the code might use the complex's base ring by default, if it's a field, rather than using QQ by default.
  • I wonder if we should deprecate the method torsion_list. Maybe this is more appropriate for a Sage worksheet than Sage source code; what do you think?

I'll keep working on this.

@jhpalmieri
Copy link
Member

Attachment: trac_15161_docstrings.patch.gz

fixes white space and other small issues in some docstrings

@jhpalmieri
Copy link
Member

comment:12

In cell_complex.py, just about everything looks good, except for one misformatted docstring (plus the missing word "as"):

diff --git a/src/sage/homology/cell_complex.py b/src/sage/homology/cell_complex.py
index d736837..02d53fa 100644
--- a/src/sage/homology/cell_complex.py
+++ b/src/sage/homology/cell_complex.py
@@ -20,7 +20,8 @@ by developers producing new classes, not casual users.
     the :meth:`~GenericCellComplex.homology` method get passed on to
     the :meth:`~GenericCellComplex.chain_complex` method and also to
     the constructor for chain complexes in
-    :class:`sage.homology.chain_complex.ChainComplex_class <ChainComplex>`, as well its
+    :class:`ChainComplex <sage.homology.chain_complex.ChainComplex_class>`,
+    as well as its
     associated
     :meth:`~sage.homology.chain_complex.ChainComplex_class.homology` method.
     This means that those keywords should have consistent meaning in

Also, the newly created files (matrix_utils.py and homology_group.py) should be added to the reference manual.

@tscrim
Copy link
Collaborator

tscrim commented Dec 1, 2013

Reviewer: John Palmieri, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Dec 1, 2013

comment:13

Hey Volker and John,

I've worked in all of John's documentation changes and made a few of my own (plus rebased to the latest version of Sage). I don't think we should deprecate torsion_list() as it's not doing any harm and someone might find (or be finding) it useful. If you're happy with my changes, then I think we can set this to positive review.

Best,

Travis


New commits:

[7c51909](https://github.com/sagemath/sagetrac-mirror/commit/7c51909)Some review changes -- code tweaks and doc formatting.
[84e22b5](https://github.com/sagemath/sagetrac-mirror/commit/84e22b5)Merge branch 'ticket/15161' into public/homology/chain_complex_parents-15161

@tscrim
Copy link
Collaborator

tscrim commented Dec 1, 2013

Changed commit from b2710cd to 7c51909

@tscrim
Copy link
Collaborator

tscrim commented Dec 1, 2013

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 1, 2013

Changed commit from 7c51909 to dd02b22

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 1, 2013

Branch pushed to git repo; I updated commit sha1. New commits:

[dd02b22](https://github.com/sagemath/sagetrac-mirror/commit/dd02b22)Fixed minor bug introduced in chain_complex_morphism.py.

@jhpalmieri
Copy link
Member

comment:15

I'd also like this change:

diff --git a/src/doc/en/reference/homology/index.rst b/src/doc/en/reference/homology/index
index 82c2f33..147cd8c 100644
--- a/src/doc/en/reference/homology/index.rst
+++ b/src/doc/en/reference/homology/index.rst
@@ -21,6 +21,8 @@ cell complexes.
    sage/homology/delta_complex
    sage/homology/cubical_complex
    sage/homology/cell_complex
+   sage/homology/homology_group
+   sage/homology/matrix_utils
    sage/interfaces/chomp
 
 .. include:: ../footer.txt

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2013

Branch pushed to git repo; I updated commit sha1. New commits:

[e955feb](https://github.com/sagemath/sagetrac-mirror/commit/e955feb)Added files to documentation and fixed internal link.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2013

Changed commit from dd02b22 to e955feb

@tscrim
Copy link
Collaborator

tscrim commented Dec 2, 2013

comment:17

Ack, I completely forgot about that. Done.

@jhpalmieri
Copy link
Member

Changed author from Volker Braun to Volker Braun, Travis Scrimshaw

@jhpalmieri
Copy link
Member

comment:18

Everything looks good to me.

@tscrim
Copy link
Collaborator

tscrim commented Dec 5, 2013

comment:19

I don't think what I did warrants author credits (especially since most of what I changes was in John's review patch :) ).

@tscrim
Copy link
Collaborator

tscrim commented Dec 5, 2013

Changed author from Volker Braun, Travis Scrimshaw to Volker Braun

@tscrim tscrim modified the milestones: sage-5.13, sage-6.0 Dec 5, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.0, sage-6.1 Dec 17, 2013
@vbraun vbraun closed this as completed in c967b64 Dec 21, 2013
@jhpalmieri
Copy link
Member

comment:22

See #15730 for a follow-up.

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