Opened 6 years ago

Closed 6 years ago

#17648 closed defect (fixed)

Move from_* functions from Partitions to Partitions_all

Reported by: ncohen Owned by:
Priority: major Milestone: sage-6.5
Component: combinatorics Keywords:
Cc: tscrim, aschilling Merged in:
Authors: Nathann Cohen Reviewers: Anne Schilling
Report Upstream: N/A Work issues:
Branch: d37d47d (Commits) Commit: d37d47d4cc77f318e8bcc21cc69558411458c248
Dependencies: Stopgaps:

Description (last modified by ncohen)

The following functions do not depend on the parameters given to the Partitions constructor: 

from_frobenius_coordinates
from_beta_numbers
from_exp
from_zero_one
from_core_and_quotient

Example: 

sage: Partitions(5,length=2).from_frobenius_coordinates(([6,3,2],[4,1,0]))
# does not read the input of Partition at all
[7, 5, 5, 1, 1]

They should belong to Partitions_all, and thise branch moves them there.

Nathann

Change History (10)

comment:1 follow-up: Changed 6 years ago by ncohen

  • Branch set to public/17648
  • Status changed from new to needs_review

comment:2 in reply to: ↑ 1 Changed 6 years ago by aschilling

Your branch is read and not viewable!

Anne

comment:3 Changed 6 years ago by git

  • Commit set to d37d47d4cc77f318e8bcc21cc69558411458c248

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

​d37d47dtrac #17648: Move from_* functions from Partitions to Partitions_all

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

Sorry, I've got an awful internet connection. Loading a page takes forever (and involves 10mn of dhcp requests)

comment:5 in reply to: ↑ 4 ; follow-up: Changed 6 years ago by aschilling

What are "those functions"? Can you be more specific in your ticket description? Also, it looks like you just moved code. Is that what you intended? No new tests etc??

comment:6 in reply to: ↑ 5 ; follow-up: Changed 6 years ago by ncohen

  • Description modified (diff)

Hello,

What are "those functions"? Can you be more specific in your ticket description?

My excuses. I thought that this would be clear from the diff, but when you reverse A and B in a file, it is true that git can chose to "remove A and add it somewhere else" or "remove B and add it somewhere else". I just updated the description to say that the three functions I moved are the following: 

from_frobenius_coordinates
from_beta_numbers
from_exp
from_zero_one
from_core_and_quotient

Also, it looks like you just moved code. Is that what you intended? No new tests etc??

I did not think that there was any need to. The doctests were correct and called Partitions().from_.... That command remains valid. On the other hand, Partition(5) is now invalid.

Nathann

Version 0, edited 6 years ago by ncohen (next)

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

Ok, this looks good. Anne

comment:8 Changed 6 years ago by aschilling

  • Reviewers set to Anne Schilling
  • Status changed from needs_review to positive_review

comment:9 Changed 6 years ago by ncohen

Thanks for the review!

Nathann

comment:10 Changed 6 years ago by vbraun

  • Branch changed from public/17648 to d37d47d4cc77f318e8bcc21cc69558411458c248
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.