Skip to content

Lazy imports with deprecation #14275

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

Closed
roed314 opened this issue Mar 14, 2013 · 18 comments
Closed

Lazy imports with deprecation #14275

roed314 opened this issue Mar 14, 2013 · 18 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Mar 14, 2013

It's annoying when other people move something around in the Sage library and it's no longer available in the previous location to be imported. Sage currently has some mechanisms for alleviating this problem (sage.misc.superceded.deprecated_callable_import and sage.structure.sage_object.register_unpickle_override for example). This ticket adds another option: lazily import the old name so that a deprecation warning is issued whenever it is referred to.

Component: misc

Author: David Roe, Travis Scrimshaw

Branch/Commit: a484809

Reviewer: Travis Scrimshaw, Nathann Cohen

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

@roed314
Copy link
Contributor Author

roed314 commented Mar 14, 2013

Attachment: 14275.patch.gz

@tscrim
Copy link
Collaborator

tscrim commented Apr 3, 2013

Work Issues: failing doctest

@tscrim
Copy link
Collaborator

tscrim commented Apr 3, 2013

comment:2

Hey David,

The doctest is failing because the mismatch of the trac numbers. Also could you add a doctest with a message as well?

Thanks,

Travis

@tscrim
Copy link
Collaborator

tscrim commented Apr 3, 2013

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Apr 22, 2013

comment:3

The way I see it, deprecation only makes sense for stuff that is imported into the global namespace. Anything that is not directly accessible from the command line is, by definition, an implementation detail. And once you require backwards compatibility for implementation details it basically becomes impossible to change anything. So there is no point in deprecating import locations.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@tscrim
Copy link
Collaborator

tscrim commented Mar 9, 2014

Commit: 108e019

@tscrim
Copy link
Collaborator

tscrim commented Mar 9, 2014

comment:6

This could be used to remove things from the global namespace but the class itself doesn't change.


New commits:

108e019Adds a deprecation option to lazy_import.

@tscrim
Copy link
Collaborator

tscrim commented Mar 9, 2014

Branch: u/tscrim/14275

@tscrim
Copy link
Collaborator

tscrim commented Mar 9, 2014

Changed work issues from failing doctest to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2014

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

a484809Doctest fixes.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 9, 2014

Changed commit from 108e019 to a484809

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 3, 2014

comment:9

Comment from the sage-combinat thread :

the way it is implemented you add a "deprecation"
feature to lazy import. But lazy imports can be used a lot, so
wouldn't it be better to instead add a "lazy" feature to
"deprecation_function_alias" ?

Nathann

@tscrim
Copy link
Collaborator

tscrim commented Apr 3, 2014

comment:10

No, because then we'd have to import deprecated_function_alias (which doesn't currently accept a message too) and import the original object just to do something like

x = deprecation_function_alias(14275, x)

Otherwise we completely change the syntax and functionality to include lazy_import. Both of which are asinine.

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 3, 2014

comment:11

Ahahaahha. Well, for as long as you know that the two reasons you gave me are bad reasons and can be easily avoided, all is fine. It is not so bad.

Good to go !

Nathann

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 3, 2014

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Nathann Cohen

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 3, 2014

Changed author from David Roe to David Roe, Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Apr 3, 2014

comment:12

Thanks Nathann.

@vbraun
Copy link
Member

vbraun commented Apr 4, 2014

Changed branch from u/tscrim/14275 to a484809

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

5 participants