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

Use native namespace package #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Dec 29, 2019

No description provided.

@isuruf
Copy link
Member Author

isuruf commented Dec 29, 2019

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

@mkoeppe
Copy link

mkoeppe commented Dec 29, 2019

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
Copy link
Member Author

isuruf commented Dec 29, 2019

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

@mkoeppe
Copy link

mkoeppe commented Dec 29, 2019

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
Copy link

mkoeppe commented Dec 29, 2019

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
Copy link
Member Author

isuruf commented Dec 29, 2019

How about changing sage_numerical_backends_coin/dependencies from

cbc | $(SAGERUNTIME) setuptools pip cython

to

cbc sagelib |  $(SAGERUNTIME) setuptools pip cython

@mkoeppe
Copy link

mkoeppe commented Dec 29, 2019

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
Copy link
Member Author

isuruf commented Dec 29, 2019

@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
Copy link
Member Author

isuruf commented Dec 29, 2019

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

@mkoeppe
Copy link

mkoeppe commented Dec 30, 2019

@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
Copy link

mkoeppe commented Dec 30, 2019

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
Copy link
Member Author

isuruf commented Dec 30, 2019

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

@mkoeppe
Copy link

mkoeppe commented Dec 30, 2019

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
Copy link

mkoeppe commented Dec 30, 2019

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 added a commit to mkoeppe/sage-numerical-backends-namespace that referenced this pull request Dec 30, 2019
@mkoeppe
Copy link

mkoeppe commented Dec 30, 2019

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 pypa/setuptools#895 (comment), I am wondering if this is really a namespace package.

@isuruf
Copy link
Member Author

isuruf commented Dec 31, 2019

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.

@mkoeppe mkoeppe force-pushed the master branch 3 times, most recently from 9ccf350 to fab98f1 Compare July 1, 2024 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants