Opened 7 years ago

Closed 6 years ago

#16403 closed defect (fixed)

Rename Graph.to_partition

Reported by: ncohen Owned by: pdehaye
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: graph theory Keywords:
Cc: sage-combinat, VivianePons, tscrim, stumpc5 Merged in:
Authors: Nathann Cohen Reviewers:
Report Upstream: N/A Work issues:
Branch: u/ncohen/16403 (Commits) Commit: 9f08c6b859a0fb853f69dbaedc73811740ee3e91
Dependencies: Stopgaps:

Description (last modified by rws)

Previous descripion was:

This function is very badly named and seems to only be there to add a statistic to find_stat, a project distinct from Sage.

ncohen: Unless there is a reason to add a function a function that associates "the partition produced by the cardinalities of the connected components of a graph" (knowing that such a function can be written with only one line of code), and unless there is a better name for it, I propose to behave as if this function should have never made it in Sage, and remove it.

I believe that this function has been added without caring at all about whether this function would be useful to other Sage users, and ONLY because of FindStat?. Unless proven otherwise, let's remove it.

As discussed there one year ago https://groups.google.com/forum/#!topic/sage-devel/SnPfidRM9j8 and independently from the rewrite promised there https://groups.google.com/forum/#!topic/sage-combinat-devel/YRc1GWa3XBg I believe that this function should be removed.

Nathann

Change History (23)

comment:1 Changed 7 years ago by ncohen

  • Branch set to u/ncohen/16403
  • Commit set to 9f08c6b859a0fb853f69dbaedc73811740ee3e91
  • Status changed from new to needs_review

New commits:

9f08c6btrac #16403: Remove Graph.to_partition

comment:2 Changed 7 years ago by pdehaye

  • Owner changed from (none) to pdehaye

comment:3 Changed 7 years ago by VivianePons

  • Cc VivianePons tscrim stumpc5 added

comment:4 follow-up: Changed 7 years ago by pdehaye

I do not understand. In the commit you are removing a function, while in the description you are saying that the function is badly named (but then, why not rename it?) and that it does not belong there (but you don't explain why, except pointing to an inconclusive long thread).

Please clarify your grounds for filing a ticket that removes someone else's work, maybe by editing the description message.

comment:5 Changed 7 years ago by ncohen

  • Description modified (diff)

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

I do not understand. In the commit you are removing a function, while in the description you are saying that the function is badly named (but then, why not rename it?) and that it does not belong there (but you don't explain why, except pointing to an inconclusive long thread).

Please clarify your grounds for filing a ticket that removes someone else's work, maybe by editing the description message.

Done. I believe that this function has been added with only FindStat?'s interest in mind, not at all Sage's users, which is what Sage is developped for. I believe that this is sufficient grounds for removing something.

If you believe that a one-line function like that is useful by itself for others, and that the authors wanted to implement this for different reasons than for their own purposes, I am eager to see how you achieve to explain that while keeping any intellectual honesty.

Please also provide a meaningful name, for the current one is not. I also personally believe that there is no good name for such a function, which is also an hint that it may be useless.

Nathann

comment:7 Changed 7 years ago by ncohen

By the way, let me add that absolutely NOBODY in the graph world will think that a 'partition' is 'a partition of an integer' and not 'a partition of the vertex set'. That's just to help you find a better name if you want to try it.

Nathann

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

Please clarify your grounds for filing a ticket that removes someone else's work, maybe by editing the description message.

Done. I believe that this function has been added with only FindStat?'s interest in mind, not at all Sage's users, which is what Sage is developped for. I believe that this is sufficient grounds for removing something.

Not done. Your first sentence still shows indecision on exactly what to do and why to do it.

If you believe that a one-line function like that is useful by itself for others, and that the authors wanted to implement this for different reasons than for their own purposes, I am eager to see how you achieve to explain that while keeping any intellectual honesty.

Why is the burden on me to explain why it is useful? The burden should be on you to first explain that this is harmful. Judging from the mailing list and what I know, I think this is useful. Be careful when publicly doubting someone's intellectual honesty. There is a fine line there. You might want to take a breather.

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

Not done. Your first sentence still shows indecision on exactly what to do and why to do it.

Are we really discussing my writing style here ?

Why is the burden on me to explain why it is useful?

Because if it is not we will remove it, like any old code we don't want to carry anymore.

The burden should be on you to first explain that this is harmful.

I do not consider this harmful, I consider this useless. We also have reasons to believe that it has been added without wondering whether it would be useful, which gives me more reasons to think that it is useless.

Judging from the mailing list and what I know, I think this is useful.

Can't do anything against pure faith. Do you only accept claims without proofs from yourself or do you intend to be as explicit as you expect me to be ? This code can be written in one line, there is no doubt on earth that the only reason to implement this function was its decorator. Besides, with a name like that nobody even knows it exists and what it does.

It does not even appear in the index of graph functions : http://www.sagemath.org/doc/reference/graphs/sage/graphs/graph.html

Be careful when publicly doubting someone's intellectual honesty. There is a fine line there. You might want to take a breather.

Man, we are discussing how bits should be arranged in a part of Sage's code that 5 people on earth know to exist. Stop taking this seriously.

Nathann

comment:10 in reply to: ↑ 9 Changed 7 years ago by pdehaye

Replying to ncohen:

Not done. Your first sentence still shows indecision on exactly what to do and why to do it.

Are we really discussing my writing style here ?

No, we are discussing what is the basis to tell a collaboration of five people that what they are doing, they are doing wrong. I am hoping I can leverage the outcome of this discussion into a similar discussion for the LMFDB project (50 people) to do things differently that what they currently are doing. Even if that does not work, you are likely to enter similar discussions when you review my students' tickets for sage in a semester. We are talking 20 or so projects, I hope. And then we can enter the same discussion again a few months later when I teach that same class scaled to Coursera level.

Because if it is not we will remove it, like any old code we don't want to carry anymore.

Who is we? You and me who rubberstamps? I want to be sure I am doing the right thing.

The burden should be on you to first explain that this is harmful.

I do not consider this harmful, I consider this useless. We also have reasons to believe that it has been added without wondering whether it would be useful, which gives me more reasons to think that it is useless.

How can you argue this is useless when people explicitly tell you this is useful to them? This might have been added because a project found it useful. A project that counts, let's say, 5 people.

Judging from the mailing list and what I know, I think this is useful.

Can't do anything against pure faith. Do you only accept claims without proofs from yourself or do you intend to be as explicit as you expect me to be ? This code can be written in one line, there is no doubt on earth that the only reason to implement this function was its decorator. Besides, with a name like that nobody even knows it exists and what it does.

How explicit do you want me to be? The claim is that people find this useless. I think the opposite and base my opinion on the fact that people on the mailing list say this is useful. Do you want me to search for specific emails? Martin R., who is not part of findstat, found this useful a few hours ago.

This code can be written in one line, and is only there for the decorator. So what? How is that bad? Abstract base classes do exist, and serve a purpose (although this was apparently very controversial when introduced).

Besides, with a name like that nobody even knows it exists and what it does.

The people in the findstat project knows that it exist and what it does, your claim is false.

Be careful when publicly doubting someone's intellectual honesty. There is a fine line there. You might want to take a breather.

Man, we are discussing how bits should be arranged in a part of Sage's code that 5 people on earth know to exist. Stop taking this seriously.

The fact is that you put my intellectual honesty in doubt,. I reiterate that this is completely inappropriate. And a very foolish way to bring lightheartedness in the discussion. I repeat that you should really take a breather if you don't see the error in that behaviour.

I might not want to be lighthearted about this, and in fact I don't want to. I just wrote an ERC grant proposal to bring the ideas of the LMFDB and the findstat project to another level, and uncertainty on a sage reviewer's behaviour is certainly not something desirable for this proposal.

comment:11 Changed 7 years ago by ncohen

  • Milestone changed from sage-6.3 to sage-duplicate/invalid/wontfix
  • Status changed from needs_review to positive_review

Okay man, let me soothe you : I have no doubts anymore on your intellectual honesty.

Go write ERC grant proposals and get famous.

Nathann

comment:12 Changed 7 years ago by pdehaye

Why do you imply my goal is to be famous? My goal is to do good research. Please quit implying motives for my actions.

comment:13 Changed 7 years ago by pdehaye

A way to do good research would be to get grant money to hire professional software engineers to actually code for sage. This is no different motive from what is done in Sydney for MAGMA, what William tries to do with the sagemathcloud or what Wolfram does with Mathematica: get money so we can hire programmers, who can code infrastructure. So mathematicians can code math more efficiently. The difference is of course in the source of the money, and as a mathematician one natural source is to write a grant. Mathematicians do that everywhere in the world, with no ulterior motive but to produce research.

There is really no direct connection between ERC grant proposals and getting famous.

Your attitude in those recent exchanges is what makes me careful in writing down why you are wrong before you clarify your argument first. The way you are conducting public scientific discourse, it can only hurt me and my ideas to engage with you scientifically before you take basic step to clean up your act. I only wrote about the ERC grant at the 12th comment to show to you that I considered this a serious matter, not for you to mock me. But of course you did see that comment through your lens, and mischaracterised what I was doing.

As it stands, the request is still there, and helping you structure thoughts that you ought to be able to structure yourself, please decide whether your argument is that (pick and choose):

  • this function body should not be in sage;
  • this function header should not be in sage;
  • the decorator should not be there.

At this stage, if you do not start acting more professionally on a website that is publicly viewable, I will do what I would do for any other website: report abuse to the owner of the website and ask for action to be taken. And of course close the ticket.

Should you think that I deserve insults or other niceties, please consider using my email instead. Please also consider that your employer probably has rules on what constitutes good scientific conduct and make sure your email fits those guidelines.

Should you want to hear more about the background for why I think you are wrong, I can either send you links to the documents I have referred (ask me by email), answer your questions (by email), we can discuss it by skype, or you could come to Zurich (I would be happy to pay for your return ticket if you pay for the ticket to come; I am unable at the moment to come to Paris due to circumstances)

comment:14 Changed 7 years ago by rws

  • Reviewers set to Ralf Stephan

Dissenting opinion, negative review of "invalid" claim: insufficient separation of 'a partition of an integer' and 'a partition of the vertex set'. Renaming would certainly be the least intrusive wrt intellectual property.

comment:15 Changed 7 years ago by rws

  • Reviewers Ralf Stephan deleted

comment:16 Changed 7 years ago by pdehaye

  • Priority changed from major to minor
  • Status changed from positive_review to needs_review

comment:17 Changed 7 years ago by pdehaye

Hi Ralf I don t get it. Who do you dissent with? Me? How can you when I haven t explained my arguments? If renaming would be least intrusive, why do you accept a ticket that removes the functionality?

comment:18 Changed 7 years ago by stumpc5

maybe to clarify: it was not Ralf giving a positive review but Nathann in comment:11.

comment:19 Changed 7 years ago by rws

  • Description modified (diff)
  • Milestone changed from sage-duplicate/invalid/wontfix to sage-6.3
  • Status changed from needs_review to needs_work
  • Summary changed from Remove Graph.to_partition to Rename Graph.to_partition

The ticket raised a point in my opinion, and what would be necessary is to change its title/description and, accordingly, the resulting code.

comment:20 Changed 7 years ago by pdehaye

Yes, thanks to both for clarifying and moving this forward.

comment:21 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:22 Changed 6 years ago by ncohen

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to positive_review

Solved in #17449

comment:23 Changed 6 years ago by vbraun

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