[sage-devel] Deleting depreciated is_functions

[Starx]

This discussion stems from:
http://groups.google.com/group/sage-devel/browse_thread/thread/979bdce4e002cd05/e8061b2ff21a4cdf?lnk=gst&q=is_AlgebraElement#e8061b2ff21a4cdf
I decided it deserves it's own thread.

The is_functions (is_Integer, is_AlgebraElement, ect) are depreciated
and have been for 4 years now. I think it's time some of them got
deleted, but I'm not sure all of them should go.

There are 260 functions defined in Sage of the form def is_Name(x)
where Name starts with a capitol letter (I didn't count the cdef
functions so there might actually be more). Of those 110 of them
simply return isinstance(x, Name) and I think those 110 can definitely
be deleted in favor of users using isinstance(x, Name). Deleting
them, replacing all their calls with calls to isinstance, and all
their imports with imports of Name is not something I'd like to do by
hand, so I'm working on a little script.

As for the other 150, some of them do the following:

def is_Name(x)
return isinstance(x, Name_something)

I didn't check but I suspect that there is a factory called Name which
is why the _whatever is there. Another example of this issue is that
is_Cone(x) returns isinstance(x, ConvexRationalPolyhedralCone) and the
function Cone constructs ConvexRationalPolyhedraCones. I'm undecided
on whether we should remove these is_functions as well. Is the user
expected to know that the factory is just a factory? and look through
the source code to figure out what the class name is?

Finally there are some is_functions that return something slightly
more complicated then a single isinstance, for example

def is_PrimeFiniteField(x):
some imports
return isinstance(x, FiniteField_prime_modn) or \
(isinstance(x, FiniteField_generic) and x.degree() == 1)

I would probably just leave these alone but I think others who've been
at this longer might have a better idea of what the best course of
action is. It seems to me that after 4 years they should either go or
they shouldn't be depreciated.

-Jim

--
Die Dunkelheit... leitet die Musik.
Die Musik... leitet die Seele.

[David Roe]

How much are these functions used in the Sage library?  I would be supportive of removing them all if possible....
David

--
To post to this group, send an email to sage-...@googlegroups.com
To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org

[Starx]

> How much are these functions used in the Sage library?

Not counting definitions, imports, or doctests, "is_[A-Z]" matches 807
times in the source. So I would definitely be writing a script to
remove whatever we decide to remove, and then make sure it builds and
tests, and read through the diff to make sure the script didn't do
anything unexpected.

-Jim

[Nicolas M. Thiery]

On Tue, Mar 27, 2012 at 10:29:12AM -0700, Starx wrote:
> ...

+1 on reducing the number of is_... functions.

> As for the other 150, some of them do the following:
>
> def is_Name(x)
> return isinstance(x, Name_something)
>
> I didn't check but I suspect that there is a factory called Name which
> is why the _whatever is there.

Precisely for this purpose, we (Florent, me, ...) are in favor, in
such situation, of merging the factory "Name" and the class
Name_something, so that one can do both:

sage: Name(...)
sage: isinstance(..., Name)

with a single gadget imported by default in sage.all.

See the discussion in ClasscallMetaclass for one way to implement
that, and IntegerRange for an example.

Also, the idiom ``is_Field(P)`` would be best replaced by ``P in Fields()``.

On a related note: with Simon King, we are in favor of keeping both
the two idioms ``P in Fields() and ``P.is_field()``, With slightly
different semantic:

- P in Fields(): is P already known to be a field? I.e. is the
category of P a subcategory of Fields()?
- P.is_field(): decide whether P is a field, possibly running a
costly computation if needed. If the answer is positive, P is
typically upgraded to the Fields() category.

Cheers,
Nicolas
--
Nicolas M. Thi�ry "Isil" <nth...@users.sf.net>
http://Nicolas.Thiery.name/

[Keshav Kini]

To respond to the OP: I'm all for getting rid of these functions.

"Nicolas M. Thiery" <Nicolas...@u-psud.fr> writes:
> Also, the idiom ``is_Field(P)`` would be best replaced by ``P in Fields()``.

As a beginner I might wonder, "Why is it `5/1 in ZZ` but `ZZ in
Fields()`? Why not `5/1 in ZZ()` or `ZZ in Fields`?"

These seeming inconsistencies definitely make learning Sage a bit
rockier of a road than it could be, I think.

Incidentally, `ZZ in Fields` works just fine::

sage: 5/1 in ZZ
True
sage: 5/1 in ZZ()
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)

/home/fs/src/workflow/<ipython console> in <module>()

TypeError: argument of type 'sage.rings.integer.Integer' is not iterable
sage: ZZ in Fields
False
sage: ZZ in Fields()
False
sage: QQ in Fields
True
sage: QQ in Fields()
True

I don't pretend to understand why this is the case :) But maybe it's
better if we tell new users to use `ZZ in Fields` instead of `ZZ in
Fields()`, to minimize confusion...? Or maybe doing so would be
misleading in ways I haven't realized?

-Keshav

----
Join us in #sagemath on irc.freenode.net !

[Florent Hivert]

Hi Keshav,

> Incidentally, `ZZ in Fields` works just fine::
>
> sage: 5/1 in ZZ
> True
> sage: 5/1 in ZZ()
> ---------------------------------------------------------------------------
> TypeError Traceback (most recent call last)
>
> /home/fs/src/workflow/<ipython console> in <module>()
>
> TypeError: argument of type 'sage.rings.integer.Integer' is not iterable
> sage: ZZ in Fields
> False
> sage: ZZ in Fields()
> False
> sage: QQ in Fields
> True
> sage: QQ in Fields()
> True
>
> I don't pretend to understand why this is the case :) But maybe it's
> better if we tell new users to use `ZZ in Fields` instead of `ZZ in
> Fields()`, to minimize confusion...? Or maybe doing so would be
> misleading in ways I haven't realized?

It works because Nicolas did the work (Ticket #9469 Category membership,
without arguments, Merged in: sage-5.0.beta6). I think you are right
saying that we should teach "QQ in Fields" to beginner rather than
"QQ in Fields()", except that if I remember correctly, there is a performance
issue for the short notation. Also they don't have the exact same meaning. The
difference is apparent in

sage: QQ^2 in VectorSpaces(QQ)
True
sage: QQ^2 in VectorSpaces
True
sage: QQ^2 in VectorSpaces(CC)
False

Note that they are still some mathematically surprising answers:

sage: QQ in VectorSpaces(QQ) # bootstrap problem
False
sage: QQbar in VectorSpaces(QQ)
False

So the answer is: thinks are going better in each new Sage release.

Cheers,

Florent

[Keshav Kini]

Florent Hivert <Florent...@lri.fr> writes:
>> sage: ZZ in Fields
>> False
>> sage: ZZ in Fields()
>> False
>> sage: QQ in Fields
>> True
>> sage: QQ in Fields()
>> True
>>
>> I don't pretend to understand why this is the case :) But maybe it's
>> better if we tell new users to use `ZZ in Fields` instead of `ZZ in
>> Fields()`, to minimize confusion...? Or maybe doing so would be
>> misleading in ways I haven't realized?
>
> It works because Nicolas did the work (Ticket #9469 Category membership,
> without arguments, Merged in: sage-5.0.beta6).

Awesome! :D Thanks, Nicolas!

> I think you are right
> saying that we should teach "QQ in Fields" to beginner rather than
> "QQ in Fields()", except that if I remember correctly, there is a performance
> issue for the short notation. Also they don't have the exact same meaning. The
> difference is apparent in
>
> sage: QQ^2 in VectorSpaces(QQ)
> True
> sage: QQ^2 in VectorSpaces
> True
> sage: QQ^2 in VectorSpaces(CC)
> False
>
> Note that they are still some mathematically surprising answers:
>
> sage: QQ in VectorSpaces(QQ) # bootstrap problem
> False
> sage: QQbar in VectorSpaces(QQ)
> False

Interesting...

> So the answer is: thinks are going better in each new Sage release.

That is all one can hope for :)

[David Kohel]

Hi,

1. (On Sage syntax):
For the record, there are some reasons why ZZ in Fields doesn't
(and perhaps shouldn't) make sense: Fields is a function not the
category it returns:

sage: Fields
<class 'sage.categories.fields.Fields'>
sage: Fields()
Category of fields

I think a student learning Sage should have a first session in which
they learn that every function takes parentheses, and that there are
some pre-defined objects at their disposal.

The ring ZZ and field QQ are pre-defined for the ease of the user,
and are not the same beasts. Is is true that ZZ(n) is valid, but is
meant to create an integer, e.g. ZZ(1). For empty argument it
defaults to ZZ(0), which allows '5/1 in ZZ()' to give an error in a
confusing place. This should not be confused with the category
constructors VectorSpaces, Fields, etc.

In the same vane as pre-defining ZZ and QQ, one could pre-define
some defaults Flds = Fields() and Vect = VectorSpaces() globally,
but unlike ZZ, QQ, RR, and CC, I think the naive or first-time user
does not usually need to have these categories at hand or need
to know anything about categories to use Sage.

Doing some magic to make 'QQ^2 in VectorSpaces' work
might just do injustice to first-time users, since it defers
the realization that they just made a typo -- unless
VectorSpaces is no longer a Python class:

sage: VectorSpaces
<class 'sage.categories.vector_spaces.VectorSpaces'>

rather an instance of the category of vector spaces.

2. (On is_Name):
Regarding is_PrimeField(F) and ilk -- remove them:
instead the syntax F.is_prime_field() is (or should be)
implemented:

sage: F = FiniteField(2)
sage: F.is_prime_field()
True
sage: F.<a> = FiniteField(8)
sage: F.is_prime_field()
False

for all fields. Where is is not, or incorrect (!):

sage: K.<a> = NumberField(x-2/3)
sage: K.is_prime_field()
False

this should be fixed. And someone should
certainly create a trac item to for this bug.

--David



On Mar 28, 11:00 am, Keshav Kini <keshav.k...@gmail.com> wrote:

[Nicolas M. Thiery]

On Wed, Mar 28, 2012 at 05:39:51AM -0700, David Kohel wrote:
> Doing some magic to make 'QQ^2 in VectorSpaces' work
> might just do injustice to first-time users, since it defers
> the realization that they just made a typo -- unless
> VectorSpaces is no longer a Python class:
>
> sage: VectorSpaces
> <class 'sage.categories.vector_spaces.VectorSpaces'>
>
> rather an instance of the category of vector spaces.

The rationale for that magic is that ``V in VectorSpaces`` is a short,
readable, and unambiguous idiom for asking whether V is a vector space
(over some field).

For parameter-less categories like Fields, I don't have a feeling for
whether one should teach new users to use the idiom ``X in Fields`` or
``X in Fields()``. I tend to use the later mysefl.

[Florent Hivert]

On Wed, Mar 28, 2012 at 11:27:24PM +0200, Nicolas M. Thiery wrote:
> On Wed, Mar 28, 2012 at 05:39:51AM -0700, David Kohel wrote:
> > Doing some magic to make 'QQ^2 in VectorSpaces' work
> > might just do injustice to first-time users, since it defers
> > the realization that they just made a typo -- unless
> > VectorSpaces is no longer a Python class:
> >
> > sage: VectorSpaces
> > <class 'sage.categories.vector_spaces.VectorSpaces'>
> >
> > rather an instance of the category of vector spaces.
>
> The rationale for that magic is that ``V in VectorSpaces`` is a short,
> readable, and unambiguous idiom for asking whether V is a vector space
> (over some field).
>
> For parameter-less categories like Fields, I don't have a feeling for
> whether one should teach new users to use the idiom ``X in Fields`` or
> ``X in Fields()``. I tend to use the later mysefl.

Speed could be a (not so important) argument.

Isn't ``X in Fields()`` faster than ``X in Fields`` ?

sage: %timeit QQ in Fields
625 loops, best of 3: 4.58 �s per loop
sage: %timeit QQ in Fields()
625 loops, best of 3: 1.67 �s per loop
sage: %timeit ZZ in Fields
625 loops, best of 3: 12.9 �s per loop
sage: %timeit ZZ in Fields()
625 loops, best of 3: 2.25 �s per loop

Cheers,

Florent

[Simon King]

Hi Florent!

On 2012-03-28, Florent Hivert <Florent...@lri.fr> wrote:
>> For parameter-less categories like Fields, I don't have a feeling for
>> whether one should teach new users to use the idiom ``X in Fields`` or
>> ``X in Fields()``. I tend to use the later mysefl.
>
> Speed could be a (not so important) argument.

Not so important??? I take speed very seriously. And by the way, testing
containment is even faster if you assign Fields() to a
constant, e.g.

sage: FieldsCat = Fields()

sage: QQ in Fields
True

sage: QQ in FieldsCat
True

sage: %timeit QQ in Fields

625 loops, best of 3: 11.4 µs per loop

sage: %timeit QQ in Fields()

625 loops, best of 3: 4.14 µs per loop
sage: %timeit QQ in FieldsCat
625 loops, best of 3: 1.86 µs per loop

Cheers,
Simon

[David Roe]

I think the idea was that if you cared about speed you would just use Fields(), but that it wasn't an important point in discussing whether QQ in Fields should be valid.
David

[Florent Hivert]

Hi Simon,

> On 2012-03-28, Florent Hivert <Florent...@lri.fr> wrote:
> >> For parameter-less categories like Fields, I don't have a feeling for
> >> whether one should teach new users to use the idiom ``X in Fields`` or
> >> ``X in Fields()``. I tend to use the later mysefl.
> >
> > Speed could be a (not so important) argument.
>
> Not so important??? I take speed very seriously. And by the way, testing
> containment is even faster if you assign Fields() to a
> constant, e.g.

I'm not saying that speed is not important in general. We where discussing
about syntax and teaching to new commers, as opposite to hardcore
optimization. In this context I don't think speed is a major
argument. Moreover, if needed, "QQ in Fields" could certainly be way optimized
(using Cython / better mro handling, etc). So speed *currently* shouldn't be
considered as a serious argument for deciding which syntax we need. After, the
proper syntax / usage is defined we certainly will think about speed.

Cheers,

Florent

[Starx]

Well, I wrote a script to delete some of the is_* functions, but it
turned out to be a little more complicated of a task then I thought it
would be, so this patch doesn't delete as many as I was aiming for but
at least it's a start: http://trac.sagemath.org/sage_trac/ticket/12824

-Jim

> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org

--

[Volker Braun]

I though the depreciation is for including is_* in the global namespace only. My impression was that they are to stay in the library code as an internal way to test that an argument is already of a suitable type. Generally, is_X(foo) is more strict than foo in X:

sage: is_Integer(5/1)

/home/vbraun/opt/sage-5.0.beta11/local/bin/sage-ipython:1: DeprecationWarning: 

Using is_Integer from the top level is deprecated since it was designed to be used by developers rather than end users.

It most likely does not do what you would expect it to do.  If you really need to use it, import it from the module that it is defined in.

  #!/usr/bin/env python

False

sage: 5/1 in ZZ

True

The latter tests that it can be converted, so you wouldn't be able to use it in ZZ's _element_constructor_(), say. Hence the need for is_Integer() in the library code.

 

[Starx]

So far this patch only deletes an is_* function if it literally does
nothing but wrap a call to isinstance without even changing the name.
If there's a change in the class name because of a factory or if the
is_* function does something more complicated like test a few
different classes then that's one thing. But to just wrap a call to
isinstance seems completely unnecessary to me, am I missing something?

-Jim

> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to
> sage-devel+...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org

--