Opened 6 years ago

Closed 6 years ago

#19886 closed defect (fixed)

remove misleading line in FormalSums

Reported by: mantepse Owned by:
Priority: major Milestone: sage-7.0
Component: algebra Keywords:
Cc: Merged in:
Authors: Martin Rubey Reviewers: Daniel Krenn
Report Upstream: N/A Work issues:
Branch: fc1a04f (Commits, GitHub, GitLab) Commit: fc1a04fec406551e6225cfa6e5cbee44bcb9f875
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

As discussed in this sage-devel discussion, the "elif" branch in this excerpt can never be visited, since if its condition was satisfied, the "if" condition would have been too, and the method would have ended with the "return" statement in the "if" branch.

class FormalSums(UniqueRepresentation, Module):

    Element = FormalSum

    def _element_constructor_(self, x, check=True, reduce=True):

        if isinstance(x, FormalSum):
            P = x.parent()
            if P is self:
                return x
            elif P == self:
                return self.element_class(x._data, check=False, reduce=False, parent=self)
            else:
                x = x._data

We therefore remove the "elif" branch which can only cause confusion.

Change History (8)

comment:1 Changed 6 years ago by mantepse

  • Branch set to u/mantepse/remove_misleading_line_in_formalsums

comment:2 Changed 6 years ago by mantepse

  • Authors set to Martin Rubey
  • Branch u/mantepse/remove_misleading_line_in_formalsums deleted
  • Component changed from PLEASE CHANGE to algebra
  • Status changed from new to needs_review

comment:3 Changed 6 years ago by mantepse

  • Branch set to u/mantepse/remove_misleading_line_in_formalsums

comment:4 Changed 6 years ago by dkrenn

  • Commit set to fc1a04fec406551e6225cfa6e5cbee44bcb9f875
  • Reviewers set to Daniel Krenn
  • Status changed from needs_review to positive_review
  • Type changed from PLEASE CHANGE to defect

LGTM


New commits:

fc1a04fremove confusing line

comment:5 Changed 6 years ago by slelievre

Can you summarize in one line in the ticket description what this ticket is about? Why was the line misleading, why removing it is making Sage better?

comment:7 Changed 6 years ago by slelievre

  • Description modified (diff)

comment:8 Changed 6 years ago by vbraun

  • Branch changed from u/mantepse/remove_misleading_line_in_formalsums to fc1a04fec406551e6225cfa6e5cbee44bcb9f875
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.