Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#14734 closed enhancement (fixed)

Map from Graphs to Partitions

Reported by: chrisjamesberg Owned by: sage-combinat
Priority: major Milestone: sage-5.11
Component: combinatorics Keywords: FindStatDays01
Cc: tscrim, VivianePons, stumpc5 Merged in: sage-5.11.beta2
Authors: Chris Berg Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Creating a method which returns the partition corresponding to the connected components of a graph.

Attachments (1)

graph_to_partition.patch (1.2 KB) - added by chrisjamesberg 6 years ago.

Download all attachments as: .zip

Change History (8)

Changed 6 years ago by chrisjamesberg

comment:1 Changed 6 years ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

Looks good. Thanks.

comment:3 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.11.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:4 Changed 6 years ago by ncohen

I had not noticed this ticket..... .....

The function's name is way too vague, and I think that it brings absolutely nothing to Sage. It is just there to add an output to find_stat, and has nothing to do in Sage.

I hate this patch.

Nathann

comment:5 Changed 6 years ago by ncohen

Oh, and just to keep on being unpleasant : from time to time I write long graph functions that I move to modules, and never import them to Graph or DiGraph? class.

I do that because we already have a lot of methods in those objects, and that they would not be useful to the vast majority of Sage users, only to specialists. So I store them where they will only be found by those who look for them.

If at the same time you add empty functions just to add maps to a third-party project, I do feel like I waste my time when I do that.

But that was just to keep on being unpleasant.

Nathann

comment:6 follow-up: Changed 6 years ago by stumpc5

I do not agree that the combinatorial_map decorator doesn't have anything to do with Sage. It organizes mathematical concepts by providing the possibility to ask for things like:

  • which involutions are there on permutations
  • which bijections between 132-avoiding permutations and Dyck paths are there,

and so on. The decorator should even be able to carry information about the maps (though this is not yet there), like "injective", "surjective", "bijective", ... And these mathematical information are interesting and useful for Sage users!

Feel free to start a discussion about the decorators on the sage-combinat mailing list, or to ask for a poll if the community wants such decorators or not. I will at least copy your comment and my answer there in order to get a discussion started.

Obviously, the decorator might have downsides. E.g., we have seen that it not yet works well together with the cached_method decorator.

I will add some further points once we have a discussion started at sage-combinat-devel.

Cheers, Christian

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

Yo !

I do not agree that the combinatorial_map decorator doesn't have anything to do with Sage.

My point in this ticket is that this very function has nothing to do in Sage.

It organizes mathematical concepts by providing the possibility to ask for things like:

  • which involutions are there on permutations
  • which bijections between 132-avoiding permutations and Dyck paths are there,

This does not mean that it HAS to be implemented as a decorator. Besides, right now the decorator seems to replace the function that is decorated by a new object, which contains the function as a method. This means that when users that never heard of find_stat run Sage commands, some computations are spent on things that are only needed for your website. Will you please acknowledge that this is not a good behaviour ? The discussion will be more interesting if you also acknowledge the problems that this thing creates.

You can build an index of these functions wherever you want, and this index will be queried whenever you really need it. You can also add information to the DOCSTRING of the methods that you find to be of interest, which will not represent any additional computations for normal users.

and so on. The decorator should even be able to carry information about the maps (though this is not yet there), like "injective", "surjective", "bijective", ... And these mathematical information are interesting and useful for Sage users!

Indeed. It would be nice. But the implementation right now is not the good one.

Feel free to start a discussion about the decorators on the sage-combinat mailing list, or to ask for a poll if the community wants such decorators or not. I will at least copy your comment and my answer there in order to get a discussion started.

I just wrote an email and sent it to a friend first. The number of people that cannot both accept to be friend and to think objectively at what they do makes me unable to talk pleasantly with most people I meet anymore.

Obviously, the decorator might have downsides. E.g., we have seen that it not yet works well together with the cached_method decorator.

I hate this decorator too. But there is sometimes a point to it. In particular, I don't see how it could be immediately improved, while I just gave you two alternatives above for this decorator.

I will add some further points once we have a discussion started at sage-combinat-devel.

I can start it. But on sage-devel. I don't see why the combinat guys should be the only ones to debate upon Sage's design choices.

Nathann

Note: See TracTickets for help on using tickets.