sagemath/sage

deprecated_function_alias should not appear in the documentation of posets

jm58660 opened this issue · 18 comments

This should not be there: http://sage-doc.sis.uta.fi/reference/combinat/sage/combinat/posets/posets.html#sage.combinat.posets.posets.FinitePoset.deprecated_function_alias

Component: documentation

Author: Jeroen Demeyer

Branch/Commit: 09c288a

Reviewer: Marc Mezzarobba

Issue created by migration from https://trac.sagemath.org/ticket/20255

comment:1

Can you more concretely give an example of something that changed, to understand better what this bug is about?

comment:2

Replying to @jdemeyer:

Can you more concretely give an example of something that changed, to understand better what this bug is about?

I installed Sage to our server just like version 7.0 before. And now see for example http://sage-doc.sis.uta.fi/reference/combinat/sage/combinat/posets/posets.html. It contains intervals() which is deprecated and also doctring for deprecated_function_alias().

comment:3

I don't believe this has changed, I think it was always like this.

Description changed:

--- 
+++ 
@@ -1 +1 @@
-In sage versions up to 7.0 deprecated functions were not visible in html documentation. In sage 7.1. they are.
+Deprecated functions should not be visible in the documentation.
comment:4

Replying to @jdemeyer:

I don't believe this has changed, I think it was always like this.

I tested with 6.10. True, except that deprecated_function_alias() was not included is posets.html.

Description changed:

--- 
+++ 
@@ -1 +1 @@
-Deprecated functions should not be visible in the documentation.
+This should not be there: http://sage-doc.sis.uta.fi/reference/combinat/sage/combinat/posets/posets.html#sage.combinat.posets.posets.FinitePoset.deprecated_function_alias
comment:6

I think that also there should not be deprecated functions visible in documentation. Or at least we should have an option to hide them, just like we already have for TESTS-blocks.

comment:7

Replying to @jm58660:

True, except that deprecated_function_alias() was not included is posets.html.

Then why not say that from the beginning if that is the real bug?

Author: Jeroen Demeyer

comment:9

Replying to @jdemeyer:

Replying to @jm58660:

True, except that deprecated_function_alias() was not included is posets.html.

Then why not say that from the beginning if that is the real bug?

Because I remembered wrong. I think that before this deprecated functions were not listed on html doc.

New commits:

09c288aDo not import deprecated_function_alias in classes

Commit: 09c288a

comment:12

Should examples at developers manual, section "Deprecation", also be changed? They kind of suggests wrong way to make a deprecation.

comment:13

Replying to @jm58660:

They kind of suggests wrong way to make a deprecation.

The intent of the documentation is clear to me (you almost never want to import something inside a class), but I'm open to suggestions.

comment:14

Replying to @jdemeyer:

Replying to @jm58660:

They kind of suggests wrong way to make a deprecation.

The intent of the documentation is clear to me (you almost never want to import something inside a class), but I'm open to suggestions.

I tried this:

diff --git a/src/doc/en/developer/coding_in_python.rst b/src/doc/en/developer/coding_in_python.rst
index 4eec8bd..2f237a7 100644
--- a/src/doc/en/developer/coding_in_python.rst
+++ b/src/doc/en/developer/coding_in_python.rst
@@ -491,19 +491,23 @@ documentation for more information on its behaviour and optional arguments.
   ``my_function(my_old_keyword=5)`` will see a warning::
 
       from sage.misc.decorators import rename_keyword
-      @rename_keyword(deprecation=666, my_old_keyword='my_new_keyword')
-      def my_function(my_new_keyword=True):
-          return my_new_keyword
+      ...
+
+          @rename_keyword(deprecation=666, my_old_keyword='my_new_keyword')
+          def my_function(my_new_keyword=True):
+              return my_new_keyword
 
 * **Rename a function/method:** call

but then syntax highlight disappeared from html ouput. So maybe we forgot this, and I try to remember right place for import next time I deprecate something.

Reviewer: Marc Mezzarobba