-
-
Notifications
You must be signed in to change notification settings - Fork 2k
update setup_helpers and a corresponding setup.py change #3206
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
Conversation
|
Ah, didn't realize. Though I've been occasionally voicing an opinion that we shouldn't have |
|
Ok, so are you ok with just adding that link back into the astropy_helpers module, @embray ? That would remove the need to update |
|
The only thing is, I'd rather keep it where it is now. What I could do (since this didn't actually intend to break anything) is the same thing I did with |
|
Basically a soft, pending deprecation/move. |
|
Pushed a few changes to astropy_helpers to fix this. |
|
ok, cool - will update this to the latest and remove the setup.py change |
defe2ba to
485be49
Compare
|
You misunderstood (or I was unclear). I suggested to not remove the setup.py change. |
|
@embray oh, gotcha, but the fixes are so that affiliated packages are still ok. See the new commit. |
|
👍 Thanks! |
|
I think this needs to be updated now to incorporate astropy/astropy-helpers#112, so it can be used by #3166 . |
|
Yes, and once this is merged I'll rebase #3166 |
|
Updated to lastest astropy-helpers. It seems to work, except now every call to |
b27897a to
0730e28
Compare
|
That's bizarre. No, I don't see anything like that. I'll try to see if I can figure out what might cause such a warning though. What Python version? |
|
I am seeing these warnings if I run |
|
Testing a change that might fix this, though it's not really a big problem. I'm still curious why you saw it by default and not me. |
|
Well, first attempt at a fix didn't do it, as it seems this is actually from deep in the import system. But it seems that adding |
|
The warnings do show up on Travis by the way, but only for Python 2.x, not 3.x. How shall we proceed, can we go ahead and merge this anyway since the warnings are harmless? |
|
Yes, I'll submit a separate PR that will fix these warnings. |
|
Ok, merging this and rebasing #3166 |
update setup_helpers and a corresponding setup.py change
|
@embray - the Note also that these are only for modules that have C code in them... Maybe this is an artifact of the And I'm seeing what @astrofrog noted on Travis: py 2.7 by default shows this, while 3.x does not. As far as I know I don't have any atypical warning settings... |
|
Yeah, this is just because prior to PEP 328 the default behavior for the import statement was to look for package-relative imports first, so if you're in Enabling |
…le that uses the import system. Motivated by astropy#3206 (comment)
|
aah, gotcha, that makes sense. Thanks for the clarification! |
|
I'm afraid I have a broken Is it something trivial I missed, or it's better to open a separate issue for it? |
|
@bsipocz Not sure if this helps, but try running |
|
So, I think the problem was that I had an obsolete |
This updates setup_helpers to the latest version to allow @astrofrog to finish up #3166 .
@embray - was it intentional to require this modification to
setup.py? Or did you not realize thatis_distutils_display_optionis also used in setup? An alternative to this PR is to putis_distutils_display_optionback intosetup_helpersin thesetup_helperspackage.See related discussions in astropy/astropy-helpers#110 and #3166