setup.py: Build op-tables.json as part of "build_py"
#1034
Conversation
mkoeppe
force-pushed
the
setup_py_build_op_tables
branch
from
1775e65
to
99536fd
Compare
Makefile
Outdated
| @@ -60,17 +60,17 @@ build: | |||
| # because pip install doesn't handle | |||
| # INSTALL_REQUIRES properly | |||
| #: Set up to run from the source tree | |||
| develop: mathics/data/op-tables.json | |||
| develop: | |||
There was a problem hiding this comment.
The removal of the dependency in here I don't understand. Why should this be removed here and below?
There was a problem hiding this comment.
It's no longer needed because setup.py now builds this file by itself
There was a problem hiding this comment.
Using (most? or all?) of the Makefile is optional.
Its purpose for old-timers like me is to allow various common tasks to be done in the same way as other projects that have Makefiles. So instead of remembering the common lingo for running tests (which has changed over time) , I can run "make check" which insteads changes from say "nosetest" to "pytest" (to whatever it is going to be next year and or next decade).
But specifically, that dependency is there to see if the optables needs to be created. Without this, whenever someone runs "make develop" the optable will get created, whether or not it was there.
There was a problem hiding this comment.
If you don't want setup.py to unconditionally generate op-tables.json, we can put in a check there; but in any case, there is no longer a need for the Makefile rule for building mathics/data/op-tables.json
There was a problem hiding this comment.
If you don't want
setup.pyto unconditionally generateop-tables.json, we can put in a check there
Done in cb38d92
There was a problem hiding this comment.
We are talking at cross purposes. The change make are mechanisms like adding Python code, or whether to use this library call, or some other and so on.
The purpose of the Makefile is to present, at least to me and other people who work across many projects on many platforms using different languages, a common interface or way to do common kinds of things in projects.
If a file is expected to get created and there is a Makefile around, then "make filename" will create the file, by whatever mechanism you want. It could do it by calling python setup.py or switch to some other "standard" mechanism in the future.
When I type "make develop", make has a built-in ability to skip steps based on tracking dependencies. You'll notice there is also a separate target for mathics/data/op-tables.json which calls the shell script that was removed. (So now this is broken with respect to the Makefile).
There was a problem hiding this comment.
I have removed the changes to the Makefile and have restored the shell script.
There was a problem hiding this comment.
Many thanks. And also thanks for improving the setup here.
I am good with this. @mmatera - you too?
|
Overall, this is great. I don't understand the changes to Makefile though. |
mkoeppe
force-pushed
the
setup_py_build_op_tables
branch
from
cb38d92
to
6557a24
Compare
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Our Mathics package is outdated (and also the interface shows test failures in CI - e.g. in https://github.com/sagemath/sage/actions/runs/7 894201915/job/21638423818#step:14:3279) Trying to update Mathics reveals more dependencies, include scikit-image and llvm-lite. Here we reduce it to a "pip" package to simplify maintenance of the package on our side. The latest released version 6.0.4 fails to install in Sage because of rigid version constraints. We are using the current HEAD version (7.0.0dev), which includes - Mathics3/mathics-core#1014 - Mathics3/mathics-core#1034 <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#37395 Reported by: Matthias Köppe Reviewer(s): Matthias Köppe, Sebastian Oehms
This makes it possible to install mathics directly from the repo, using
pip install git+https://github.com/Mathics3/mathics-coreAlso updating the definition of the Python packages in
pyproject.toml.