Ticket #11244 (closed defect: fixed)

Opened 2 years ago

Last modified 19 months ago

In python-2.7 deprecation warnings are not shown to the user by default

Reported by: fbissey Owned by: jason
Priority: major Milestone: sage-4.7.2
Component: misc Keywords:
Cc: kini Work issues:
Report Upstream: N/A Reviewers: Mariah Lenox
Authors: François Bissey Merged in: sage-4.7.2.alpha0
Dependencies: Stopgaps:

Description (last modified by fbissey) (diff)

In python 2.7 deprecation warnings are a developer only feature. Which mean that sage's deprecation warnings are not shown either. A previous patch in #9958 only re-enable deprecation warnings in doctests but not in regular sage sessions.

Apply:

Attachments

trac_11244_reenable_deprecationwarnings_in_python27.patch Download (1.0 KB) - added by fbissey 2 years ago.
new patch to restore deprecation warnings in python 2.7 (now in git format)
trac_11244_fix_combinatpartition_warnings.patch Download (1.3 KB) - added by fbissey 2 years ago.
Patch to reenable extra deprecation warning in sage/combinat/partition.py.
trac_11244_fixmoredeprecationswarnings.patch Download (3.2 KB) - added by fbissey 2 years ago.
3 more files fixed. double messaging removed in two of them. [rebased on 4.7.alpha5]
trac_11244_fixmoredeprecationswarnings.2.patch Download (2.5 KB) - added by fbissey 2 years ago.
Use this patch if you base your review on 4.7.1.alpha1

Change History

comment:1 Changed 2 years ago by fbissey

  • Status changed from new to needs_review
  • Description modified (diff)
  • Authors set to François Bissey

Changed 2 years ago by fbissey

new patch to restore deprecation warnings in python 2.7 (now in git format)

comment:2 Changed 2 years ago by fbissey

  • Status changed from needs_review to needs_work

There is an issue with

sage -t -force_lib "devel/sage-main/sage/combinat/partition.py"
**********************************************************************
File "/usr/share/sage/devel/sage-main/sage/combinat/partition.py", line 2511:
    sage: RestrictedPartitions(5,[3,2,1])
Expected:
    doctest:...: DeprecationWarning: RestrictedPartitions is deprecated; use Partitions with the parts_in keyword instead.
    doctest:...: DeprecationWarning: RestrictedPartitions_nsk is deprecated; use Partitions with the parts_in keyword instead.
    Partitions of 5 restricted to the values [1, 2, 3]
Got:
    doctest:260: DeprecationWarning: RestrictedPartitions_nsk is deprecated; use Partitions with the parts_in keyword instead.
    Partitions of 5 restricted to the values [1, 2, 3] 

But clearly I am not sure why the original code should give both a warning and a deprecation warning. Of course my patch only let sage display the later.

comment:3 Changed 2 years ago by fbissey

The code in question

    import warnings
    warnings.warn('RestrictedPartitions is deprecated; use Partitions with the parts_in keyword instead.', DeprecationWarning, stacklevel=2)
    from sage.misc.misc import deprecation
    deprecation('RestrictedPartitions is deprecated; use Partitions with the parts_in keyword instead.')
    return RestrictedPartitions_nsk(n, S, k)

comment:4 Changed 2 years ago by fbissey

  • Description modified (diff)
  • Work issues set to fix warnings in sage/combinat/partition.py

I see that in fact there are are two related functions being deprecated and this is a way to warn for both at the same time.

comment:5 Changed 2 years ago by fbissey

  • Status changed from needs_work to needs_review
  • Description modified (diff)

Changed 2 years ago by fbissey

Patch to reenable extra deprecation warning in sage/combinat/partition.py.

comment:6 Changed 2 years ago by fbissey

  • Description modified (diff)

Fixing my mistaken assumptions about warnings in general.

comment:7 Changed 2 years ago by fbissey

  • Status changed from needs_review to needs_work

There are more deprecation warnings issued by the standard python mechanism rather than sage's mechanism

sage-4.7.alpha1 $ grep -r DeprecationWarning\,\ stacklevel *
sage/matrix/constructor.py:                    warnings.warn("invocation of block_matrix with just a list whose length is a perfect square is deprecated. See the documentation for details.", DeprecationWarning, stacklevel=2)
sage/combinat/partition.py:    warnings.warn('RestrictedPartitions is deprecated; use Partitions with the parts_in keyword instead.', DeprecationWarning, stacklevel=2)
sage/combinat/partition.py:        warnings.warn('RestrictedPartitions_nsk is deprecated; use Partitions with the parts_in keyword instead.', DeprecationWarning, stacklevel=2)
sage/groups/perm_gps/permgroup.py:        warnings.warn('quotient_group() is deprecated; use quotient() instead.', DeprecationWarning, stacklevel=2)
sage/graphs/base/graph_backends.py:                    DeprecationWarning, stacklevel=2)
sage/graphs/base/graph_backends.py:                    DeprecationWarning, stacklevel=2)
sage/misc/misc.py:    warn(message, DeprecationWarning, stacklevel=3)

comment:8 Changed 2 years ago by fbissey

  • Description modified (diff)

Changed 2 years ago by fbissey

3 more files fixed. double messaging removed in two of them. [rebased on 4.7.alpha5]

comment:9 Changed 2 years ago by fbissey

  • Status changed from needs_work to needs_review

Changed 2 years ago by fbissey

Use this patch if you base your review on 4.7.1.alpha1

comment:10 Changed 2 years ago by fbissey

  • Description modified (diff)

comment:11 Changed 2 years ago by mariah

  • Status changed from needs_review to positive_review
  • Reviewers set to Mariah Lenox

Patches applied to sage-4.7.1.alpha2, then did 'sage -b', followed by 'make testlong'. All tests passed. Positive review!

comment:12 Changed 2 years ago by fbissey

Thank you Mariah for the review. Thank you John for the suggestion to make it better. Thank you Steve for pointing the obvious.

Thank you all for making this reach this stage.

comment:13 Changed 2 years ago by jdemeyer

  • Milestone changed from sage-4.7.1 to sage-4.7.2

comment:14 Changed 2 years ago by fbissey

  • Work issues fix warnings in sage/combinat/partition.py deleted

Removing the work issue as it was in fact dealt with.

comment:15 Changed 22 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-4.7.2.alpha0

comment:16 Changed 22 months ago by jason

(for another ticket) maybe it would be best if we subclassed the warnings to all inherit from a SageWarning? class, and then turn on just Sage warnings...

comment:17 Changed 19 months ago by kini

  • Cc kini added
Note: See TracTickets for help on using tickets.