Skip to content

Fix dynamic versioning#1209

Merged
damonge merged 21 commits intomasterfrom
dynamic_versioning
Nov 13, 2024
Merged

Fix dynamic versioning#1209
damonge merged 21 commits intomasterfrom
dynamic_versioning

Conversation

@damonge
Copy link
Collaborator

@damonge damonge commented Nov 8, 2024

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

@coveralls
Copy link

coveralls commented Nov 8, 2024

Pull Request Test Coverage Report for Build 11815531052

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 97.413%

Files with Coverage Reduction New Missed Lines %
pyccl/pk2d.py 1 98.72%
Totals Coverage Status
Change from base Build 11146463072: -0.05%
Covered Lines: 6513
Relevant Lines: 6686

💛 - Coveralls

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why, but I wasn't able to fix dynamic versioning without this requirement.


setup(
distclass=Distribution,
setup_requires=['setuptools_scm'],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main thing that fixes dynamic versioning.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you still require setup.py and not use just pyproject.toml?

Copy link
Collaborator Author

@damonge damonge Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do use pyproject.toml, but this is the only way I have found to make sure that the dynamic version numbers are assigned.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

dVda[i] * self._mf[..., :] * _selm[..., :],
self._lmass
)
).squeeze()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Collaborator

@carlosggarcia carlosggarcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments

- pyyaml
- numpy
# The below is only because the currnt version of fast-pt uses deprecated scipy functions.
- numpy<2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked that this doesn't break other CCL stuff that might have been updated to numpy 2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also want to add this numpy<2 in the pyproject.toml dependencies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


setup(
distclass=Distribution,
setup_requires=['setuptools_scm'],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it go here or in the dependencies section below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry done

Copy link
Collaborator

@carlosggarcia carlosggarcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@damonge damonge merged commit 4b9cb93 into master Nov 13, 2024
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.

pyccl version in pypi and conda outdated SWIG 4.3.0 is not compatible

3 participants