Opened 7 years ago

Closed 7 years ago

#12961 closed defect (fixed)

Documentation of the facade argument in Poset contructor

Reported by: ncohen Owned by: sage-combinat
Priority: major Milestone: sage-5.1
Component: combinatorics Keywords: posets
Cc: nthiery Merged in: sage-5.1.beta1
Authors: Nathann Cohen Reviewers: Nicolas M. Thiéry
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Attachments (1)

trac_12961.patch (6.5 KB) - added by ncohen 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by ncohen

  • Status changed from new to needs_review

comment:2 Changed 7 years ago by nthiery

  • Authors set to Nathann Cohen
  • Keywords posets added
  • Reviewers set to Nicolas M. Thiéry

Hi Nathann,

I did a couple changes here and there. All tests pass. If you are happy with the changes, please set a positive review!

comment:3 follow-up: Changed 7 years ago by ncohen

Hellooooooo Nicolas !!

Well, I think that if I read the docstring about "facade" in its current state I would not think that this may be the reason why the types of my elements are changed. I believe it would be better to let the explanation about what this parameter does (which you improved yourself) stay near the description of the "facade" parameter, otherwise the message by itself is not that clear....

What do you think ?

Oh, and I should be in Orsay around the end of next week. Will you be back in France by then ? :-)

Nathann

comment:4 in reply to: ↑ 3 ; follow-up: Changed 7 years ago by nthiery

Replying to ncohen:

Well, I think that if I read the docstring about "facade" in its current state I would not think that this may be the reason why the types of my elements are changed. I believe it would be better to let the explanation about what this parameter does (which you improved yourself) stay near the description of the "facade" parameter, otherwise the message by itself is not that clear....

What do you think ?

I did not want to have examples or something too long in the INPUT description, otherwise it's hard getting an overview. Now, feel free to add a short link "See rubric ... below for details" though.

Oh, and I should be in Orsay around the end of next week. Will you be back in France by then ? :-)

Back and away again! I am leaving Wednesday.

But if you come to Orsay in September, we will have plenty of time to work together :-)

Cheers,

Nicolas

Changed 7 years ago by ncohen

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

description, otherwise it's hard getting an overview. Now, feel free to add a short link "See rubric ... below for details" though.

I just did that : added "see below for details" at the end of the 'facade' entry. If this patch is fine for you, it is for me too ! ;-)

Back and away again! I am leaving Wednesday.

Arg !!! Ok, ok.. Perhaps Florent will be there then ! ;-)

But if you come to Orsay in September, we will have plenty of time to work together :-)

Oh, so you will be back by then ? :-D

Nathann

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

Replying to ncohen:

I just did that : added "see below for details" at the end of the 'facade' entry. If this patch is fine for you, it is for me too ! ;-)

Sounds good to me. Positive review (I am running the tests now)

Cheers,

Nicolas

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

Replying to nthiery:

Sounds good to me. Positive review (I am running the tests now)

Tests passed.

comment:8 Changed 7 years ago by ncohen

  • Status changed from needs_review to positive_review

Well then... :-)

Nathann

comment:9 Changed 7 years ago by jdemeyer

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