Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate the old .field() method from polyhedron class #22551

Closed
jplab opened this issue Mar 8, 2017 · 17 comments
Closed

Deprecate the old .field() method from polyhedron class #22551

jplab opened this issue Mar 8, 2017 · 17 comments

Comments

@jplab
Copy link

jplab commented Mar 8, 2017

Ticket #11634 made the ppl library be the default backend for the Polyhedron class.

Currently, Polyhedron objects still have a .field() method which is ill-named and since #11634 out-dated.

This ticket deprecates this method.

CC: @mo271 @mkoeppe @videlec @sagetrac-tmonteil @fchapoton

Component: geometry

Keywords: days84

Author: Jean-Philippe Labbé

Branch/Commit: 6291dd1

Reviewer: Vincent Delecroix

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

@jplab jplab added this to the sage-7.6 milestone Mar 8, 2017
@jplab
Copy link
Author

jplab commented Mar 8, 2017

Branch: u/jipilab/deprecate_field

@jplab
Copy link
Author

jplab commented Mar 8, 2017

New commits:

1e26fe9Deprecate old keyword and method field

@jplab
Copy link
Author

jplab commented Mar 8, 2017

Commit: 1e26fe9

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

Changed branch from u/jipilab/deprecate_field to none

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

Changed commit from 1e26fe9 to none

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

comment:3

In cdd_convert you need to test the deprecation

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

New commits:

1e26fe9Deprecate old keyword and method field

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

Branch: u/jipilab/deprecate_field

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

Commit: 1e26fe9

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

comment:5
@@ -2032,8 +2032,6 @@ class Polyhedron_base(Element):
         """
         return self.parent().base_ring()
 
-    field = base_ring
-
     @cached_method
     def center(self):

No deprecation here?

@videlec
Copy link
Contributor

videlec commented Mar 8, 2017

Reviewer: Vincent Delecroix

@jplab
Copy link
Author

jplab commented Mar 8, 2017

comment:7

Replying to @videlec:

@@ -2032,8 +2032,6 @@ class Polyhedron_base(Element):
         """
         return self.parent().base_ring()
 
-    field = base_ring
-
     @cached_method
     def center(self):

No deprecation here?

Hmm. Right. I confused the keyword with the method here. I will add a deprecation warning.

@jplab
Copy link
Author

jplab commented Mar 8, 2017

comment:8

Replying to @videlec:

In cdd_convert you need to test the deprecation

Since it is in a nested function, I removed the deprecation warning.

This was intended merely to make the nomenclature about base ring consistent across the geometry component...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2017

Changed commit from 1e26fe9 to 6291dd1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 8, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

6291dd1Added and removed deprecation warnings

@videlec
Copy link
Contributor

videlec commented Mar 10, 2017

comment:11

I can not reproduce the timeout from the patchbot.

@vbraun
Copy link
Member

vbraun commented Mar 11, 2017

Changed branch from u/jipilab/deprecate_field to 6291dd1

@vbraun vbraun closed this as completed in 8bc23ff Mar 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants