Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename Graph.to_partition #16403

Closed
nathanncohen mannequin opened this issue May 27, 2014 · 22 comments
Closed

Rename Graph.to_partition #16403

nathanncohen mannequin opened this issue May 27, 2014 · 22 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented May 27, 2014

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

CC: @sagetrac-sage-combinat @VivianePons @tscrim @stumpc5

Component: graph theory

Author: Nathann Cohen

Branch/Commit: u/ncohen/16403 @ 9f08c6b

Issue created by migration from https://trac.sagemath.org/ticket/16403

@nathanncohen nathanncohen mannequin added this to the sage-6.3 milestone May 27, 2014
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

Commit: 9f08c6b

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

New commits:

9f08c6btrac #16403: Remove Graph.to_partition

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

Branch: u/ncohen/16403

@nathanncohen nathanncohen mannequin added the s: needs review label May 27, 2014
@pdehaye pdehaye mannequin self-assigned this May 27, 2014
@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 27, 2014

comment:4

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.

@nathanncohen

This comment has been minimized.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

comment:6

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

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

comment:7

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

@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 27, 2014

comment:8

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

comment:9

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

@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 27, 2014

comment:10

Replying to @nathanncohen:

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.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented May 27, 2014

comment:11

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

Go write ERC grant proposals and get famous.

Nathann

@nathanncohen nathanncohen mannequin removed this from the sage-6.3 milestone May 27, 2014
@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 27, 2014

comment:12

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

@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 27, 2014

comment:13

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)

@rwst
Copy link

rwst commented May 28, 2014

Reviewer: Ralf Stephan

@rwst
Copy link

rwst commented May 28, 2014

comment:14

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.

@rwst
Copy link

rwst commented May 28, 2014

Changed reviewer from Ralf Stephan to none

@pdehaye pdehaye mannequin added p: minor / 4 and removed p: major / 3 labels May 28, 2014
@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 28, 2014

comment:17

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?

@stumpc5
Copy link
Contributor

stumpc5 commented May 28, 2014

comment:18

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

@rwst

This comment has been minimized.

@rwst
Copy link

rwst commented May 28, 2014

comment:19

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

@rwst rwst added this to the sage-6.3 milestone May 28, 2014
@rwst rwst changed the title Remove Graph.to_partition Rename Graph.to_partition May 28, 2014
@pdehaye
Copy link
Mannequin

pdehaye mannequin commented May 28, 2014

comment:20

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

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jan 21, 2015

comment:22

Solved in #17449

@nathanncohen nathanncohen mannequin removed this from the sage-6.4 milestone Jan 21, 2015
@vbraun vbraun closed this as completed Jan 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants