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

absolute_import in Cython files #22808

Closed
jdemeyer opened this issue Apr 14, 2017 · 23 comments
Closed

absolute_import in Cython files #22808

jdemeyer opened this issue Apr 14, 2017 · 23 comments

Comments

@jdemeyer
Copy link

Depends on #22806

CC: @fchapoton

Component: python3

Author: Jeroen Demeyer

Branch/Commit: ac72aa1

Reviewer: Frédéric Chapoton

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

@jdemeyer jdemeyer added this to the sage-8.0 milestone Apr 14, 2017
@fchapoton
Copy link
Contributor

comment:1

one step in #22975

@fchapoton
Copy link
Contributor

comment:2

next step in #23068

@fchapoton
Copy link
Contributor

comment:3

next step in #23589

@fchapoton
Copy link
Contributor

comment:4

next step in #23641

@fchapoton
Copy link
Contributor

comment:5

next step in #23662

@fchapoton
Copy link
Contributor

comment:6

maybe the last step in #23745

but how can we check ?

@fchapoton
Copy link
Contributor

comment:7

Jeroen, do you have an idea how to find places where adding "absolute_import" is still needed ?

@jdemeyer
Copy link
Author

comment:8

Do you mean for Python and Cython code in the Sage library or only Cython code?

@fchapoton
Copy link
Contributor

comment:9

Well, only Cython, to deal with this ticket. But of course one should also check for py files.

@jdemeyer
Copy link
Author

comment:10

OK, both Python and Cython. I think it is best to treat Python and Cython the same here. Imports work the same way. The only difference is that Cython also has cimport.

I think that I only created this ticket because at some point you were adding from future import absolute_import to a lot of Python files but not to Cython files. It seemed like you overlooked Cython files.

@fchapoton
Copy link
Contributor

comment:11

So what should we do here ?

I would like to have some certitude before closing, but I do not know how to test that everything is in order.

@jdemeyer
Copy link
Author

comment:12

Actually, it might be easy to force Cython to use absolute_import everywhere. Maybe we can try the same thing with print_function.

@fchapoton
Copy link
Contributor

comment:13

Replying to @jdemeyer:

Actually, it might be easy to force Cython to use absolute_import everywhere. Maybe we can try the same thing with print_function.

any idea on how to do that ?

@jdemeyer
Copy link
Author

comment:14

Replying to @fchapoton:

any idea on how to do that ?

Yes.

@jdemeyer
Copy link
Author

@jdemeyer
Copy link
Author

Commit: ac72aa1

@jdemeyer
Copy link
Author

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

comment:16

Just one change required...


New commits:

ac72aa1Fix relative cimport

@fchapoton
Copy link
Contributor

comment:17

Great !
Does this mean that you tried to compile while forcing cython to use absolute import, and this was the only failure ?

@jdemeyer
Copy link
Author

comment:18

Replying to @fchapoton:

Great !
Does this mean that you tried to compile while forcing cython to use absolute import, and this was the only failure ?

Yes. I didn't run doctests, but at least built and ran Sage.

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:19

Then let us consider that this ticket is fixed. Thanks !

@fchapoton fchapoton modified the milestones: sage-8.0, sage-8.1 Nov 14, 2017
@vbraun
Copy link
Member

vbraun commented Dec 11, 2017

Changed branch from u/jdemeyer/absolute_import_in_cython_files to ac72aa1

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