Opened 6 years ago

Closed 6 years ago

#11300 closed enhancement (fixed)

Add get_part method to partition.py

Reported by: jbandlow Owned by: sage-combinat
Priority: major Milestone: sage-4.7.1
Component: combinatorics Keywords: partitions, days30
Cc: sage-combinat Merged in: sage-4.7.1.alpha1
Authors: Jason Bandlow Reviewers: Florent Hivert
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Add a method to partitions to return a default value for parts that do not exist.

sage: p = Partition([2,1])
sage: p.get_part(0)
2
sage: p.get_part(5)
0

Apply: trac_11300-get_part_partition-jb.2.patch

Attachments (2)

trac_11300-get_part_partition-jb.patch (1.7 KB) - added by jbandlow 6 years ago.
trac_11300-get_part_partition-jb.2.patch (1.7 KB) - added by jbandlow 6 years ago.

Download all attachments as: .zip

Change History (13)

Changed 6 years ago by jbandlow

comment:1 Changed 6 years ago by jbandlow

  • Authors set to jbandlow
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by jbandlow

  • Description modified (diff)

comment:3 Changed 6 years ago by hivert

  • Authors changed from jbandlow to Jason Bandlow
  • Keywords days30 added; sagedays30 removed
  • Reviewers set to Florent Hivert
  • Status changed from needs_review to positive_review

Looks good and is useful !

comment:4 follow-up: Changed 6 years ago by mhansen

  • Status changed from positive_review to needs_info

The one issue with this is the following:

sage: p = Partition([4,3])
sage: type(p.get_part(0))
<type 'sage.rings.integer.Integer'>
sage: type(p.get_part(10))
<type 'int'>

Is there a use case for a changeable default value?

comment:5 in reply to: ↑ 4 Changed 6 years ago by hivert

Replying to mhansen:

The one issue with this is the following:

> sage: p = Partition([4,3])
> sage: type(p.get_part(0))
> <type 'sage.rings.integer.Integer'>
> sage: type(p.get_part(10))
> <type 'int'>

That is a very good remark !

Is there a use case for a changeable default value?

If yes this can be corrected by changing default=0 to default=Integer(0).

comment:6 Changed 6 years ago by jbandlow

I've changed the default value to Integer(0). I don't have a specific use case for another value, but it seems like it might come in handy. Do you have a reason you think it's a bad idea? I could probably be convinced to get rid of it.

comment:7 follow-up: Changed 6 years ago by saliola

Hello Jason. I'm curious: did you compare timings of using the try-except statement:

 	768	        try: 
 	769	            return self[i] 
 	770	        except IndexError: 
 	771	            return default

with an if statement, like this one:

    if i < len(self._list):
        return self._list[i]
    else: 
        return default

I would guess that try-except statements are slower, but I have not tried it.

Changed 6 years ago by jbandlow

comment:8 in reply to: ↑ 7 Changed 6 years ago by jbandlow

  • Status changed from needs_info to needs_review

Replying to saliola:

Hello Jason. I'm curious: did you compare timings of using the try-except statement: with an if statement, like this one:

I would guess that try-except statements are slower, but I have not tried it.

I tried it and you're right--'if' is significantly faster. I've replaced the patch.

comment:9 Changed 6 years ago by saliola

  • Status changed from needs_review to positive_review

Thanks for testing that. Positive review.

comment:10 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:11 Changed 6 years ago by jdemeyer

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