sagemath / sage-numerical-backends-coin

COIN-OR mixed integer linear programming backend for SageMath. Source repository for https://pypi.org/project/sage-numerical-backends-coin/, can be installed on top of distributions providing SageMath. See also https://github.com/sagemath/sage-numerical-backends-gurobi and https://github.com/sagemath/sage-numerical-backends-cplex
GNU General Public License v2.0
3 stars 4 forks source link

Use native namespace package #4

Open isuruf opened 4 years ago

isuruf commented 4 years ago

Since the file sage/numerical/backends/__init__.py always exists, this works in python 2 too.

mkoeppe commented 4 years ago

This looks great. But apart from the Python 3 dependence that I don't want at this point, another problem is the following: sagelib, in src/setup.py (clean_stale_files), tries to be helpful and removes unknown files in its installation tree (site-packages/sage).

isuruf commented 4 years ago

Does that happen if you use pip to install this package?

mkoeppe commented 4 years ago

In the sage build system, whenever a package is installed (e.g., using sage -i), sagelib is installed again. I think this invokes clean_stale_files after completing the build.

mkoeppe commented 4 years ago

I suppose I could do both - install sage_numerical_backends_coin.coin_backend (which would survive) and install a namespace package sage.numerical.backends.coin_backend (which would be nuked by the sagelib install) that just re-exports. This would eliminate the monkeypatching mentioned in the README.

isuruf commented 4 years ago

How about changing sage_numerical_backends_coin/dependencies from

cbc | $(SAGERUNTIME) setuptools pip cython

to

cbc sagelib |  $(SAGERUNTIME) setuptools pip cython
mkoeppe commented 4 years ago

How about changing sage_numerical_backends_coin/dependencies ... to

cbc $(SAGERUNTIME) | setuptools pip cython

This kind of works but the sagelib target (a dependency by SAGERUNTIME) is always rebuilt, and so sage_numerical_backends_coin would be rebuilt on every make build as well.

isuruf commented 4 years ago

@mkoeppe, that's correct though. When you make a change in sagelib, you need sage_numerical_backends_coin to be rebuilt as it depends on GenericBackend

isuruf commented 4 years ago

Since the build directories are deleted, this would rebuild the package from sctrach. @embray, is there a way to avoid this?

mkoeppe commented 4 years ago

@mkoeppe, that's correct though. When you make a change in sagelib, you need sage_numerical_backends_coin to be rebuilt as it depends on GenericBackend

In https://git.sagemath.org/sage.git/commit/?id=127989764d490e3d9359712970923bd993b14067 I have added the specific .pxd (and .h) files of the sage sources as dependencies of the new packages, keeping $(SAGERUNTIME) as an order-only dependency. This seems to work well. Thanks a lot for the helpful discussion!

mkoeppe commented 4 years ago

Since the build directories are deleted, this would rebuild the package from sctrach. @embray, is there a way to avoid this?

With the fine-grained dependencies that I just declared, I think the recompiles of the packages will be quite limited, so we don't seem to need build system innovations here.

isuruf commented 4 years ago

There's still the issue about cleaning up as you mentioned

mkoeppe commented 4 years ago

There's still the issue about cleaning up as you mentioned

Yes, I have created https://trac.sagemath.org/ticket/28925 ("Modify clean_stale_files to support modularization of sagelib by namespace packages") for this.

mkoeppe commented 4 years ago

I suppose I could do both - install sage_numerical_backends_coin.coin_backend (which would survive) and install a namespace package sage.numerical.backends.coin_backend (which would be nuked by the sagelib install) that just re-exports. This would eliminate the monkeypatching mentioned in the README.

I will provide the re-exporting namespace package in the new package https://github.com/mkoeppe/sage-numerical-backends-namespace

mkoeppe commented 4 years ago

I have set it up now, using the same structure as in your pull request. It works, except for setup.py build giving the message:

package init file 'sage/numerical/backends/__init__.py' not found (or not a regular file)

(not an error). Reading https://github.com/pypa/setuptools/issues/895#issuecomment-268771045, I am wondering if this is really a namespace package.

isuruf commented 4 years ago

It works, except for setup.py build giving the message:

This is wrong though. That package uses 3 different modules sage.numerical.backends.coin_backend, sage.numerical.backends.cplex_backend, `sage.numerical.backends.gurobi_backend, but sagelib has some other modules in the same level. A package should have one top level module that is not shared by other packages.