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

convert some imports in combinat to absolute imports #20617

Closed
fchapoton opened this issue May 18, 2016 · 29 comments
Closed

convert some imports in combinat to absolute imports #20617

fchapoton opened this issue May 18, 2016 · 29 comments

Comments

@fchapoton
Copy link
Contributor

Let us try to start using the absolute import everywhere.

Component: python3

Author: Frédéric Chapoton

Branch/Commit: 7d586f0

Reviewer: Jeroen Demeyer

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

@fchapoton fchapoton added this to the sage-7.3 milestone May 18, 2016
@fchapoton
Copy link
Contributor Author

Commit: 4a03357

@fchapoton
Copy link
Contributor Author

Branch: public/20617

@fchapoton
Copy link
Contributor Author

New commits:

4a03357convert some imports in combinat to absolute imports

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2016

Changed commit from 4a03357 to 1c74237

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2016

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

1c74237more import changes in combinat

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2016

Changed commit from 1c74237 to 3b5e552

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2016

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

3b5e552another import change in combinat/all

@jdemeyer
Copy link

comment:4

Would it not be better to add from future import absolute_import then?

@fchapoton
Copy link
Contributor Author

comment:5

I thought this was not necessary since python 2.7.

Or maybe you mean, to avoid some backward move, like people re-introducing old-style imports ?

@jdemeyer
Copy link

comment:6

Replying to @fchapoton:

Or maybe you mean, to avoid some backward move, like people re-introducing old-style imports ?

Yes exactly.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 19, 2016

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

2a0ed11trac 20617 adding future import absolute_import

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 19, 2016

Changed commit from 3b5e552 to 2a0ed11

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 19, 2016

Changed commit from 2a0ed11 to 5add1af

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 19, 2016

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

5add1aftrac 20617 better placement of future import

@fchapoton
Copy link
Contributor Author

comment:9

ok, this one does pass the tests (except for the 4 failures coming from somewhere else, see #20529, which prevents my patchbot to work smoothly)

@jdemeyer
Copy link

comment:10

I'm sorry but I keep seeing strange things in the files that you update...

What is the point of __doc__ = r""" instead of a normal docstring?

@fchapoton
Copy link
Contributor Author

comment:11

no idea. This was there before, maybe this can stay ?

@jdemeyer
Copy link

comment:12

See #20633 for the silly __doc__ stuff.

Suggestion: leave those modules alone in this ticket.

@fchapoton
Copy link
Contributor Author

comment:13

why ? ticket #20633 can wait for its turn to come, no ?

@jdemeyer
Copy link

comment:14

If it's easy to make two tickets not conflict, I think that's the best way.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2016

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

23b5775removing the future where conflict may happen with other ticket

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2016

Changed commit from 5add1af to 23b5775

@jdemeyer
Copy link

comment:16

Two more things:

  1. Are you sure you need those from . import all statements? I don't see the point.

  2. In src/sage/combinat/__init__.py, can you remove those commented-out imports?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2016

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

7d586f0trac 20617, details after reviewer's comments

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 20, 2016

Changed commit from 23b5775 to 7d586f0

@jdemeyer
Copy link

comment:18

positive_review modulo the patchbot.

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@fchapoton
Copy link
Contributor Author

comment:19

ok, patchbot is "green", modulo the usual 4 failing doctests from #20529

setting to positive review. Thanks !

@vbraun
Copy link
Member

vbraun commented May 21, 2016

Changed branch from public/20617 to 7d586f0

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