Fix dynamic versioning #1209
Fix dynamic versioning #1209
Conversation
Pull Request Test Coverage Report for Build 11815531052Details
💛 - Coveralls |
pyccl/numpy.i
Outdated
There was a problem hiding this comment.
SWIG changed and this requires a new version of numpy.i (see here)
| @@ -1,4 +1,4 @@ | |||
| cmake_minimum_required(VERSION 3.2) | |||
| cmake_minimum_required(VERSION 3.5) | |||
There was a problem hiding this comment.
I am not sure why, but I wasn't able to fix dynamic versioning without this requirement.
| @@ -63,5 +63,6 @@ def run(self): | |||
|
|
|||
| setup( | |||
| distclass=Distribution, | |||
| setup_requires=['setuptools_scm'], | |||
There was a problem hiding this comment.
This is the main thing that fixes dynamic versioning.
There was a problem hiding this comment.
why do you still require setup.py and not use just pyproject.toml?
There was a problem hiding this comment.
We do use pyproject.toml, but this is the only way I have found to make sure that the dynamic version numbers are assigned.
There was a problem hiding this comment.
ok. weird. I think in TJPCov we have dynamic versioning and works fine with just pyproject.toml. I couldn't see any obvious difference that might cause this, though.
There was a problem hiding this comment.
Interesting. Well, if you can ever look into it, that'll be great. For now this is as far as I can get, and I think it is relatively urgent that we fix this! Our conda and pip releases are severely out of date.
| @@ -186,7 +186,7 @@ def number_counts(self, cosmo, *, selection, | |||
| mint[i] = self._integrator( | |||
| dVda[i] * self._mf[..., :] * _selm[..., :], | |||
| self._lmass | |||
| ) | |||
| ).squeeze() | |||
There was a problem hiding this comment.
A few warnings were being triggered because of this (the result of the integral is an array of size 1 instead of a scalar, which numpy doesn't like).
| @@ -9,8 +9,9 @@ dependencies: | |||
| - fftw | |||
| - swig | |||
| - pyyaml | |||
| - numpy | |||
| # The below is only because the currnt version of fast-pt uses deprecated scipy functions. | |||
| - numpy<2 | |||
There was a problem hiding this comment.
Have you checked that this doesn't break other CCL stuff that might have been updated to numpy 2?
There was a problem hiding this comment.
The opposite, actually. We cannot yet move to numpy 2 because many of our dependencies still don't use numpy 2. firecrown is in the same situation. This will be a tricky migration.
There was a problem hiding this comment.
You also want to add this numpy<2 in the pyproject.toml dependencies.
| @@ -63,5 +63,6 @@ def run(self): | |||
|
|
|||
| setup( | |||
| distclass=Distribution, | |||
| setup_requires=['setuptools_scm'], | |||
There was a problem hiding this comment.
why do you still require setup.py and not use just pyproject.toml?
pyproject.toml
Outdated
| @@ -1,5 +1,5 @@ | |||
| [build-system] | |||
| requires = ["setuptools>=64", "setuptools_scm>=8", "cmake", "swig"] | |||
| requires = ["setuptools>=64", "setuptools_scm>=8", "cmake", "swig", "numpy<2"] | |||
There was a problem hiding this comment.
Would it go here or in the dependencies section below?
Our latest releases haven't been pushed to pypi and conda because the automatic versioning was not working. This was broken in #1144
Unfortunately fully testing this will require us to publish a new release and see if it pulls through. Hopefully we only need to do this once.
Closes #1207
Closes #1204