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

BLD: remove __NUMPY_SETUP__ from builtins at end of setup.py #7956

Merged
merged 1 commit into from
Sep 3, 2016

Conversation

rgommers
Copy link
Member

@rgommers rgommers commented Aug 21, 2016

May help reduce the occurrence of gh-2434.

@JensTimmerman
Copy link

JensTimmerman commented Aug 22, 2016

I just tested this and this seems to fix my problem.

the current numpy fails on this (the build of h5py fails, because it believes numpy isn't completely installed yet, since __NUMPY_SETUP__ is still defined)
setup.py:

 from setuptools import setup                                                                                             
 PACKAGE = {                                                                                                              
     'name': 'numpytest',                                                                                                 
     'version': '0.7.1',                                                                                                  
     'install_requires': [                                                                                                
         'numpy >= 1.10.0',                                                                                               
         'matplotlib >= 1.3.1',                                                                                           
         'h5py',                       
     ],        
   # Workaround from                                                                                                    
   # https://github.com/numpy/numpy/issues/2434#issuecomment-65252402                                                   
   # and                                                                                                                
   # https://github.com/h5py/h5py/issues/535#issuecomment-79158166                                                
  # this makes the installation work on the 2nd try
   'setup_requires': [                                                                                                  
        'numpy >= 1.10.0',                                                                                                
     ],                                                                                                                   
     'author': 'jenstimmerman@gmail.com',                                                                                 

 }                                                                                                                        
setup(**PACKAGE)  

your branch builds fine:
setup.py:

from setuptools import setup                                                                                             
PACKAGE = {                                                                                                              
   'name': 'numpytest',                                                                                                 
   'version': '0.7.1',                                                                                                  
   'install_requires': [                                                                                                
       'numpy >= 1.12.0.dev0+940247',                                                                                  
       'matplotlib >= 1.3.1',                                                                                           
       'h5py',                          
   ],                                                                                                                   
   # this workaround is still needed in this test, because h5py would pull in numpy 1.11.1, and not this branch yet. Can be removed once h5py depends on numpy >=1.12.1?      
  # but now the install works the first time around!             
   'setup_requires': [                                                                                                  
       'numpy >= 1.12.0.dev0+940247',                                                                                  
   ],                                                                                                                   
   'author': 'jenstimmerman@gmail.com',                                                                                 
   'dependency_links': [                                                                                               
         "git+https://github.com/rgommers/numpy.git@numpysetup#egg=numpy-1.12.0",    # this is how we tell setuptools where to find this branch for testing
   ]                                                                                                                   
}                                                                                                                        
setup(**PACKAGE)       

I test this with python setup.py test and remove your .eggs folder afterwards, because the first setup.py script will work on the second run, because the second time around __NUMPY_SETUP__ is not set again.

also make sure to have run easy_install --user cython, or the above setup.py scripts will fail, but this is a different issue, see #7959

I can also confirm that
easy_install --user https://github.com/rgommers/numpy/archive/numpysetup.zip
and
setup.py install --user
from this branch still work as expected.

I had some issues testing bdist_rpm (also on master, so this means nothing)

@rgommers
Copy link
Member Author

Thanks @JensTimmerman. I'll try to reproduce your issue first, but looks like this is a correct change then.

@charris
Copy link
Member

charris commented Aug 22, 2016

It will be interesting to see if this affects matplotlib. They explicitly delete __NUMPY_SETUP__ and this is likely to raise a NameError. Of course, they should not be deleting private variables...

@charris
Copy link
Member

charris commented Aug 22, 2016

NVM, MPL checks for the attribute before deleting it.

@JensTimmerman
Copy link

@charris matplotlib is included in my testcases and works.

pip install --user https://github.com/rgommers/numpy/archive/numpysetup.zip also works fine

@charris charris added this to the 1.11.2 release milestone Aug 25, 2016
@charris
Copy link
Member

charris commented Aug 25, 2016

@rgommers I'm going to try to put this in 1.11.2rc1 if you are OK with that and accept it in time. That way it will get more testing and it doesn't look to break anything common at the moment.

@embray
Copy link
Contributor

embray commented Aug 30, 2016

I don't know that this fixes the specific issue I described, but it might help with other similar issues. It's not a bad idea anyhow.

@charris
Copy link
Member

charris commented Sep 2, 2016

@rgommers A comment to clarify the reason for the deletion would be helpful. If none is forth coming, I'm going to put this in this weekend in any case, so complain now or never.

@rgommers
Copy link
Member Author

rgommers commented Sep 3, 2016

I'm going to try to put this in 1.11.2rc1 if you are OK with that and accept it in time

sounds good to me.

A comment to clarify the reason for the deletion would be helpful.

good point, will add

@rgommers
Copy link
Member Author

rgommers commented Sep 3, 2016

I don't know that this fixes the specific issue I described, but it might help with other similar issues. It's not a bad idea anyhow.

I agree, it's only a partial fix. There are a number of ways for the install to fail with the same symptoms. This one is the most straightforward to fix.

@rgommers
Copy link
Member Author

rgommers commented Sep 3, 2016

OK did some more testing/checking, this should be OK to merge once TravisCI is happy.

@charris charris merged commit a08ed82 into numpy:master Sep 3, 2016
@charris
Copy link
Member

charris commented Sep 3, 2016

Thanks Ralf.

@charris charris removed this from the 1.11.2 release milestone Sep 3, 2016
@rgommers rgommers deleted the numpysetup branch September 3, 2016 21:35
@rgommers
Copy link
Member Author

rgommers commented Sep 3, 2016

Thanks @JensTimmerman for pointing out this issue and testing.

@mhvk
Copy link
Contributor

mhvk commented Sep 4, 2016

@MSeifert04 - our problem was actually fixed already, by #7941

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants