Opened 11 years ago

Closed 11 years ago

# poset top() function breaks when top element has boolean value False

Reported by: Owned by: niles sage-combinat major sage-4.7 combinatorics beginner, poset, top niles, novoselt sage-4.7.alpha4 Niles Johnson Andrey Novoseltsev N/A

### Description

Posets determine whether there is a top element by converting the element returned by `._hasse_diagram.top()` to a boolean. Instead, the function should check whether the returned value is `None`.

Here is the current definition of `.top()`:

```    def top(self):
"""
Returns the top element of the poset, if it exists.

EXAMPLES::

sage: P = Poset({0:[3],1:[3],2:[3],3:[4,5],4:[],5:[]})
sage: P.top() is None
True
sage: Q = Poset({0:[1],1:[]})
sage: Q.top()
1
"""
hasse_top = self._hasse_diagram.top()
if hasse_top:
return self._vertex_to_element(hasse_top)
else:
return None
```

And here is an example showing how it fails:

```sage: p = Poset([[0],[]]); p
Finite poset containing 1 elements
sage: p.list()
[0]
sage: p.top()
sage: p.top() is None
True
sage: p._hasse_diagram.top()
0
sage: p._hasse_diagram.top() is None
False
```

### comment:1 Changed 11 years ago by novoselt

• Milestone set to sage-4.7

### comment:2 Changed 11 years ago by niles

• Authors set to Niles Johnson
• Status changed from new to needs_review

patched; written to parallel the code for `.bottom()`.

### comment:3 follow-up: ↓ 4 Changed 11 years ago by novoselt

Looks good, but it would be nice to mention ticket number in the TESTS ;-)

### comment:4 in reply to: ↑ 3 Changed 11 years ago by niles

Looks good, but it would be nice to mention ticket number in the TESTS ;-)

Well, actually I've thought about this before -- I think there was a time when the developer guide recommended it. But when I looked into this a few months ago, I couldn't find the recommendation any more, and someone convinced me to drop it with something like the following: Wouldn't it be strange and pointless if, many years from now, the Sage source code were filled with references to ticket numbers that are meaningless to most users?

Do you really think there's a compelling reason to mention the ticket?

### comment:5 follow-up: ↓ 6 Changed 11 years ago by novoselt

The fifth bullet in http://sagemath.org/doc/developer/trac.html#section-review-patches

I don't have a strong opinion on it and was only mentioning it since it is required. (I have also realized that I never marked such doctests with the inline comment and I am pretty sure not all bugs that I have reviewed used them...)

As for meaningless references, perhaps it isn't very relevant since most of such "marked" tests should go into TESTS section and not EXAMPLES, so only developers will look at them. On the other hand, I didn't ever have a need to follow these references. But that may be because I am not a sufficiently active developer ;-)

### comment:6 in reply to: ↑ 5 Changed 11 years ago by niles

The fifth bullet in http://sagemath.org/doc/developer/trac.html#section-review-patches

Well, I stand corrected -- somehow I can't find that list when I'm looking for it; probably because I associate it with reviewing patches rather than creating them.

...

Oh, how embarrassing: checking back, it seems that I *wrote* that line (#9771). So the new patch adds the ticket number :)

thanks for keeping me honest,

Niles

### comment:7 Changed 11 years ago by novoselt

• Reviewers set to Andrey Novoseltsev
• Status changed from needs_review to positive_review

Well, that was a long time ago ;-) Positive review!

:) thanks!

### comment:9 Changed 11 years ago by jdemeyer

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