Opened 9 years ago

Closed 8 years ago

#10764 closed task (fixed)

Cleanup a few Cython warnings

Reported by: robertwb Owned by: GeorgSWeber
Priority: major Milestone: sage-5.2
Component: build Keywords:
Cc: Merged in: sage-5.2.beta0
Authors: François Bissey, André Apitzsch Reviewers: François Bissey, André Apitzsch
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by aapitzsch)

See, e.g. the list of warnings at #10493.

Apply attachment:trac_10764.patch

Attachments (3)

trac_10764-fix_deprecation_warning.patch (1.9 KB) - added by fbissey 9 years ago.
Fix deprecation warnings (updated after #10258)
trac_10764-fix-gen_interpreters_doctest.patch (775 bytes) - added by fbissey 9 years ago.
previous patch broke a doctest in gen_interpreters.py.
trac_10764.patch (11.0 KB) - added by aapitzsch 8 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by fbissey

  • Milestone changed from sage-4.6.2 to sage-4.7

I am starting the work by fixing the deprecation warnings. One of the easy bits :)

comment:2 Changed 9 years ago by fbissey

I hadn't mentioned these in #10493 I assume we want to fix them as well:

warning: sage/gsl/fft.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/gsl/gsl_array.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/modular/modsym/p1list.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/modules/free_module_element.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/modules/free_module_element.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/laurent_series_ring_element.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polydict.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_gf2x.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_zmod_flint.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_element.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_integer_dense_ntl.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_zz_pex.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_integer_dense_flint.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_modn_dense_ntl.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_modn_dense_ntl.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_modn_dense_ntl.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/structure/parent.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead
warning: sage/rings/polynomial/polynomial_rational_flint.pyx:1:0: __getslice__, __setslice__, and __delslice__ are not supported by Python 3, use __getitem__, __setitem__, and __delitem__ instead

comment:3 Changed 9 years ago by fbissey

I am looking at gsl/fft.pyx and some of the code is strange

    def __getitem__(self, size_t i):
        if i < 0 or i >= self.n:
            raise IndexError
        cdef int j
        j = 2*i
        return self.data[2*i], self.data[2*i+1]

    def __getslice__(self, Py_ssize_t i, Py_ssize_t j):
        # Todo -- make this actually fast.
        return list(self)[i:j]

There is already a getitem, I don't think it would work with slices. But getitem and getslice look very different. One asks for size_t while the other asks for Py_ssize_t I assume one is deprecated. The code in getitem is very strange. j is declared, computed and never used, the code also return something different from you would naively expect. None of this seem to ever be called from anywhere else in sage.

comment:4 Changed 9 years ago by fbissey

The warning in sage/combinat/combinat_cython.pyx has been fixed as a side effect of #10258. Updated patch coming shortly.

Changed 9 years ago by fbissey

Fix deprecation warnings (updated after #10258)

Changed 9 years ago by fbissey

previous patch broke a doctest in gen_interpreters.py.

comment:5 follow-up: Changed 8 years ago by aapitzsch

The patch attached to #12041 should fix some warnings.

comment:6 in reply to: ↑ 5 Changed 8 years ago by aapitzsch

Replying to aapitzsch:

The patch attached to #12041 should fix some warnings.

#12093 fixes the remaining ones. See also #12119.

comment:7 Changed 8 years ago by aapitzsch

  • Status changed from new to needs_review

The other deprecation warnings are fixed, so this ticket can be reviewed.

comment:8 Changed 8 years ago by aapitzsch

  • Status changed from needs_review to needs_work
  • Work issues set to Rebase

Please rebase to current beta.

Changed 8 years ago by aapitzsch

comment:9 Changed 8 years ago by aapitzsch

  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues Rebase deleted

comment:10 Changed 8 years ago by aapitzsch

patchbot: apply trac_10764.patch

comment:11 follow-up: Changed 8 years ago by fbissey

sorry to have been out of touch. I assume your patch is a rebase of my 2 patches, correct?

comment:12 in reply to: ↑ 11 Changed 8 years ago by aapitzsch

Replying to fbissey:

sorry to have been out of touch. I assume your patch is a rebase of my 2 patches, correct?

Yes. Additionally I removed some trailing white spaces.

comment:13 Changed 8 years ago by fbissey

  • Authors set to François Bissey, André Apitzsch
  • Milestone changed from sage-5.1 to sage-5.2
  • Reviewers set to François Bissey, André Apitzsch
  • Status changed from needs_review to positive_review

I am putting this to positive review. I guess we agree on what needs doing and that it doesn't have side effects.

comment:14 Changed 8 years ago by jdemeyer

  • Summary changed from Cleanup cython warnings to Cleanup a few Cython warnings

comment:15 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.2.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.