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

Clean imports related to coding theory #19315

Closed
sagetrac-dlucas mannequin opened this issue Sep 30, 2015 · 20 comments
Closed

Clean imports related to coding theory #19315

sagetrac-dlucas mannequin opened this issue Sep 30, 2015 · 20 comments

Comments

@sagetrac-dlucas
Copy link
Mannequin

sagetrac-dlucas mannequin commented Sep 30, 2015

For now, a lot of modules are imported from sage.coding when Sage starts.
These modules can be lazy imported instead for better performances.

In all.py there's also several deprecated imports related to a 2-year old ticket.
These deprecation warnings can be removed too.

CC: @nathanncohen

Component: coding theory

Author: David Lucas

Branch: bc6ecf9

Reviewer: Vincent Delecroix, Jeroen Demeyer

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

@sagetrac-dlucas sagetrac-dlucas mannequin added this to the sage-6.9 milestone Sep 30, 2015
@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 22, 2015

Branch: u/dlucas/clean_imports

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 22, 2015

comment:3

I removed deprecation warnings in all.py, passed everything in lazy imports and fixed a few doctests that broke in the operation.


New commits:

8e42c26Removed deprecation warnings and lazy imported everything

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 22, 2015

Commit: 8e42c26

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 22, 2015

Author: David Lucas

@sagetrac-dlucas sagetrac-dlucas mannequin modified the milestones: sage-6.9, sage-6.10 Oct 22, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 23, 2015

Changed commit from 8e42c26 to dadb126

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 23, 2015

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

dadb126Removed lazy import for self_dual_codes_binary

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 23, 2015

comment:5

As putting self_dual_codes_binary in a lazy import seemed to mess up with the garbage collector doctests in categories/homset.py and structure/coerce.pyx I changed it for a hard import instead.

@videlec
Copy link
Contributor

videlec commented Oct 29, 2015

comment:6

Salut David,

  1. The function cyclotomic_cosets was deprecated in cyclotomic_cosets for any finite ring #16464 which is also more than one year old. Could you remove the function and the import as well?

  2. Do you really want to keep codesize_upper_bound, dimension_upper_bound, etc in the global namespace?

Vincent

@videlec
Copy link
Contributor

videlec commented Oct 29, 2015

Reviewer: Vincent Delecroix

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 29, 2015

comment:7

Hello Vincent,

The function cyclotomic_cosets was deprecated in #16464 which is also more than one year old. Could >you remove the function and the import as well?

Sure, I'll do that.

Do you really want to keep codesize_upper_bound, dimension_upper_bound, etc in the global namespace?

Not really. I can put them in a catalog so one could access them typing sage.codes.bounds or something like that. It would be better. But if I do that, I guess I need to put a new deprecation warning over these bounds, isn't it?

David

@jdemeyer
Copy link

comment:8

Can you please also remove the ugly backslashes, they are not needed inside (...) or [...].

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2015

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

9110eafRemoved deprecated cyclotomic_cosets
bc6ecf9New catalog for bounds. Some changes in all.py, fixed doctests.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2015

Changed commit from dadb126 to bc6ecf9

@sagetrac-dlucas
Copy link
Mannequin Author

sagetrac-dlucas mannequin commented Oct 29, 2015

comment:10

I removed cyclotomic_cosetsmethod, add a new catalog for bounds so they won't be available in the global namespace... When the new deprecation warning I added will be deleted in a year. I fixed the doctests that went down in the process.

I also removed the backslashes.

@videlec
Copy link
Contributor

videlec commented Oct 30, 2015

comment:11

I am good with this ticket. Jeroen?

@videlec
Copy link
Contributor

videlec commented Oct 30, 2015

Changed reviewer from Vincent Delecroix to Vincent Delecroix, ​Jeroen Demeyer

@jdemeyer
Copy link

comment:12

I haven't actually reviewed, I just looked a diff and saw too many backslashes hurting my eyes :-)

@vbraun
Copy link
Member

vbraun commented Oct 30, 2015

Changed branch from u/dlucas/clean_imports to bc6ecf9

@jdemeyer
Copy link

Changed reviewer from Vincent Delecroix, ​Jeroen Demeyer to Vincent Delecroix, Jeroen Demeyer

@jdemeyer
Copy link

Changed commit from bc6ecf9 to none

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