Graph(....,format='elliptic_curve_congruence')

54 views
Skip to first unread message

Nathann Cohen

unread,
Oct 9, 2015, 3:23:56 AM10/9/15
to Sage devel
Hello everybody,

It is possible to build a graph with the following syntax:

Graph(something, format="elliptic_curve_congruence")

I will attempt to refactor Graph.__init__ today, and I am a bit
embarassed by the corresponding block of code (~30 lines, at line
1602) in graph.py that deals with elliptic curves (no doctest in
graph.py).

Could this block of code be moved elsewhere, in a file that deals with
elliptic curves?

This Graph.__init__ function is appallingly long, and I will precisely
try to split it into subfunctions.

Thanks,

Nathann

Nathann Cohen

unread,
Oct 9, 2015, 4:52:13 AM10/9/15
to Sage devel
P.S.: It seems that this code is not called by *any* part of Sage:

(read|…)~/sage$ grep "elliptic_curve_congruence" . -R
./graphs/graph.py: - ``'elliptic_curve_congruence'`` - data must be an
./graphs/graph.py: elif format == 'elliptic_curve_congruence':

No doctest, no other function calls it.

Nathann

John Cremona

unread,
Oct 9, 2015, 9:40:17 AM10/9/15
to SAGE devel
Disclaimer -- I did not wrote that code and did not know it was there!

I agree that having this hidden away in graphs.py is not sensible, and
having it in src/sage/schemes/elliptic_curves/ is more sensible.

How shall we do this? Would it work to make that a separate ticket
from anything else you are doing, Nathann?

John
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To post to this group, send email to sage-...@googlegroups.com.
> Visit this group at http://groups.google.com/group/sage-devel.
> For more options, visit https://groups.google.com/d/optout.

Nathann Cohen

unread,
Oct 9, 2015, 9:44:07 AM10/9/15
to Sage devel
> Disclaimer -- I did not wrote that code and did not know it was there!

Same here :-P

> I agree that having this hidden away in graphs.py is not sensible, and
> having it in src/sage/schemes/elliptic_curves/ is more sensible.
>
> How shall we do this? Would it work to make that a separate ticket
> from anything else you are doing, Nathann?

Yes of course. The refactoring ticket I opened will already be
sufficiently hard to review.

How do you want to see it moved there? Should we keep the syntax
Graph(....,format="elliptic_curve_congruence") or would it be better
as some_object.graph()?

Or will it become an individual function that turns some input into a graph?

It's mostly up to you, given that I do not even understand what this
code is about ^^;

Nathann

John Cremona

unread,
Oct 9, 2015, 10:01:19 AM10/9/15
to SAGE devel
On 9 October 2015 at 09:44, Nathann Cohen <nathan...@gmail.com> wrote:
>> Disclaimer -- I did not wrote that code and did not know it was there!
>
> Same here :-P
>
>> I agree that having this hidden away in graphs.py is not sensible, and
>> having it in src/sage/schemes/elliptic_curves/ is more sensible.
>>
>> How shall we do this? Would it work to make that a separate ticket
>> from anything else you are doing, Nathann?
>
> Yes of course. The refactoring ticket I opened will already be
> sufficiently hard to review.
>
> How do you want to see it moved there? Should we keep the syntax
> Graph(....,format="elliptic_curve_congruence") or would it be better
> as some_object.graph()?

I can do that independently, if it will not cause conflicts with your
ongoing work.

The function's argument is a list of objects (elliptic curves over Q)
rather than one object, so it cannot be a method of some elliptic
curve class but will have to be stand-alone. Still, it will be more
visible than at present since it will at least appear in the reference
manual in an elliptic curve page where people who might use it might
actually see it!

I will make a ticket for this now.

>
> Or will it become an individual function that turns some input into a graph?
>
> It's mostly up to you, given that I do not even understand what this
> code is about ^^;
>
> Nathann
>

Nathann Cohen

unread,
Oct 9, 2015, 10:04:01 AM10/9/15
to Sage devel
> I can do that independently, if it will not cause conflicts with your
> ongoing work.

Works for me. You can base your branch on whatever you are using right
now, and I will rebase it (if needed) atop of my refactoring. There
will probably be a conflict, but very a probably one that is solved
easily.

Thanks,

Nathann

John Cremona

unread,
Oct 9, 2015, 10:04:47 AM10/9/15
to SAGE devel
On 9 October 2015 at 10:00, John Cremona <john.c...@gmail.com> wrote:
> On 9 October 2015 at 09:44, Nathann Cohen <nathan...@gmail.com> wrote:
>>> Disclaimer -- I did not wrote that code and did not know it was there!
>>
>> Same here :-P
>>
>>> I agree that having this hidden away in graphs.py is not sensible, and
>>> having it in src/sage/schemes/elliptic_curves/ is more sensible.
>>>
>>> How shall we do this? Would it work to make that a separate ticket
>>> from anything else you are doing, Nathann?
>>
>> Yes of course. The refactoring ticket I opened will already be
>> sufficiently hard to review.
>>
>> How do you want to see it moved there? Should we keep the syntax
>> Graph(....,format="elliptic_curve_congruence") or would it be better
>> as some_object.graph()?
>
> I can do that independently, if it will not cause conflicts with your
> ongoing work.
>
> The function's argument is a list of objects (elliptic curves over Q)
> rather than one object, so it cannot be a method of some elliptic
> curve class but will have to be stand-alone. Still, it will be more
> visible than at present since it will at least appear in the reference
> manual in an elliptic curve page where people who might use it might
> actually see it!
>
> I will make a ticket for this now.

http://trac.sagemath.org/ticket/19382
Reply all
Reply to author
Forward
0 new messages