Opened 10 years ago

Closed 10 years ago

#13119 closed defect (fixed)

._apply_module_morphism() in combinat/ doesn't handle zero element properly

Reported by: zabrocki Owned by: sage-combinat
Priority: minor Milestone: sage-5.6
Component: combinatorics Keywords:
Cc: saliola@… Merged in: sage-5.6.beta2
Authors: Mike Zabrocki Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by tscrim)

a potential defect in ._apply_module_morphism function calls list( self.basis().keys() ) when self.is_zero() is True. This works fine if self.basis() is a finite set, but causes a problem if self.basis() is an infinite set with an iteration function. In this case, the function hangs trying to list the infinite set.

Apply only: trac_13119_apply_module_morphism_bugfix-mz.patch

Attachments (1)

trac_13119_apply_module_morphism_bugfix-mz.patch (3.8 KB) - added by zabrocki 10 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 10 years ago by zabrocki

I added a patch to this ticket. Discussion is at

I changed one thing in addition since I had originally reported this bug. That is, if the codomain is not known and it looks for the parent of an element of the image to determine the codomain then raise a value error if the element returned by the function doesn't have a parent.

comment:2 Changed 10 years ago by zabrocki

  • Status changed from new to needs_review

comment:3 Changed 10 years ago by zabrocki

Removed a bit of whitespace within the function

Apply: trac_13119_apply_module_morphism_bugfix-mz.2.patch

Version 0, edited 10 years ago by zabrocki (next)

comment:4 Changed 10 years ago by tscrim

  • Description modified (diff)
  • Reviewers set to Travis Scrimshaw

Hey Mike,

Two minor things:

  • Could you use the auto-linking :func:`on_basis` in the docstring?
  • Since at some point, there is a shift to Python 3, I think we should start using that syntax:
    raise ValueError('Codomain could not be determined')

Looks good to me otherwise.


For patchbot:

Apply only: trac_13119_apply_module_morphism_bugfix-mz.patch

comment:5 Changed 10 years ago by zabrocki

Hi Travis, Thanks for picking this up. I made your changes and caught one or two more fixes in the docstring. -Mike

comment:6 Changed 10 years ago by tscrim

  • Status changed from needs_review to positive_review

Looks good. Thanks Mike.

comment:7 Changed 10 years ago by jdemeyer

  • Merged in set to sage-5.6.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.