gh-1159: Add CosmologyWithOmegaM protocol#1162
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a more specific typing protocol (CosmologyWithOmegaM) to represent cosmology objects that provide Omega_m(z), and uses it where Omega_m is actually required (e.g., density-based shell weights). This refines type contracts in GLASS without changing runtime behavior.
Changes:
- Introduce
CosmologyWithOmegaMprotocol inglass.cosmology. - Update
DensityWeightto requireCosmologyWithOmegaMinstead of the broaderCosmology. - Adjust the corresponding unit test type annotation for
DensityWeight.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
glass/cosmology.py |
Adds the new CosmologyWithOmegaM protocol and removes HasOmegaM from the base Cosmology protocol. |
glass/shells.py |
Narrows DensityWeight.cosmo to CosmologyWithOmegaM and updates the attribute docstring accordingly. |
tests/core/test_shells.py |
Updates test_density_weight to require a cosmology object that supports Omega_m. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
glass/shells.py:153
- Renaming the dataclass field from
cosmotocosmo_omega_mchanges the public constructor/attribute API (e.g., existingDensityWeight(cosmo=...)calls would break) and is inconsistent with the other weight classes in this module which all usecosmo. It should remaincosmoand only the type should be narrowed toCosmologyWithOmegaM. Also, the__call__docstring currently says “comoving distance” even though this class is a density weight.
cosmo_omega_m: CosmologyWithOmegaM
def __call__(self, z: FloatArray) -> FloatArray:
"""
Uniform weight in comoving distance.
connoraird
left a comment
There was a problem hiding this comment.
Similar to my comments on other PRs, should we be updating the fixture type for test_density_weight to be CosmologyWithOmegaM?
I'm not 100% sure, this is what I originally had, but Copilot said to change it. |
|
@connoraird realise now I hadn't made a new fixture, the latest change does that now so hopefully passes. |
Description
Adds a
CosmologyWithOmegaMprotocol asOmegaMisn't used that often.Closes: #1159
Changelog entry
Added: A
CosmologyWithOmegaMprotocolChecks