pycopanlpjml compatibility#204
Open
jnnsbrr wants to merge 27 commits into
Open
Conversation
The __init__ method sets self._default_value but the property was incorrectly accessing self._default, causing AttributeError when copying Variables.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #204 +/- ##
=========================================
Coverage ? 53.55%
=========================================
Files ? 74
Lines ? 3262
Branches ? 0
=========================================
Hits ? 1747
Misses ? 1515
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR modernizes the repository’s Python tooling (Black/flake8/pytest + CI), reformats many modules and templates accordingly, and adds an initial pytest suite intended to validate core data_model and model_components behavior.
Changes:
- Add pytest-based unit tests and a test configuration (
tests/,pytest.ini,pyproject.toml). - Introduce Black + flake8 configuration and apply broad formatting/cleanup across code, templates, studies, and docs.
- Add/adjust CI workflows for checks and releases, plus RTD + flake8 config files.
Reviewed changes
Copilot reviewed 112 out of 114 changed files in this pull request and generated 48 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_working_functionality.py | New “known working” smoke tests |
| tests/test_data_model.py | New unit tests for data_model |
| tests/test_basic_functionality.py | New basic import/functionality tests |
| tests/conftest.py | New shared pytest fixtures |
| templates/studies/plot_trajectory.py | Black/PEP8-style formatting |
| templates/studies/SOME_STUDY.py | Black/PEP8-style formatting |
| templates/models/SOME_MODEL.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/model.py | Template import formatting + class decl cleanup |
| templates/model_components/SOME_COMPONENT/interface.py | Template formatting + comment wrapping |
| templates/model_components/SOME_COMPONENT/implementation/world.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/social_system.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/metabolism.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/individual.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/environment.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/culture.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/cell.py | Template formatting cleanup |
| templates/model_components/SOME_COMPONENT/implementation/init.py | Template formatting cleanup |
| studies/seven_dwarfs/run_seven_dwarfs.py | Formatting + minor style normalization |
| studies/esd_description_paper_example/run_esd_example.py | Formatting + line wrapping |
| studies/esd_description_paper_example/plot_figure5.py | Formatting + line wrapping |
| scripts/update_citation.py | Removed release helper script |
| scripts/release.sh | Removed release helper script |
| pytest.ini | Simplified pytest configuration |
| pyproject.toml | Add Black/flake8/pytest config + dev deps changes |
| pycopancore/util/seeding.py | Formatting cleanup |
| pycopancore/util/functions.py | Remove unused imports + formatting |
| pycopancore/util/init.py | Formatting + safer exception handling |
| pycopancore/runners/hooks.py | Formatting + minor robustness tweaks |
| pycopancore/runners/init.py | Switch to docstring + add __all__ |
| pycopancore/process_types/step.py | Formatting cleanup |
| pycopancore/process_types/implicit.py | Formatting cleanup |
| pycopancore/process_types/explicit.py | Formatting cleanup |
| pycopancore/process_types/event.py | Formatting cleanup |
| pycopancore/process_types/init.py | Reorder imports + add __all__ |
| pycopancore/process_types/ODE.py | Formatting cleanup |
| pycopancore/private/_trajectory_dictionary.py | Formatting + clearer imports |
| pycopancore/private/_mixin.py | Change metaclass attribute handling + formatting |
| pycopancore/private/_abstract_runner.py | Minor formatting |
| pycopancore/private/_abstract_process_taxon_mixin.py | Formatting + clearer assertions |
| pycopancore/private/_abstract_process.py | Minor formatting |
| pycopancore/private/_abstract_entity_mixin.py | Formatting + quote normalization |
| pycopancore/private/init.py | Replace empty docstring |
| pycopancore/models/seven_dwarfs.py | Formatting + class declaration cleanup |
| pycopancore/models/base_only.py | Minor formatting |
| pycopancore/models/_testing/generic_imitation/simple.py | Formatting + config dict normalization |
| pycopancore/models/_testing/generic_imitation/other_component.py | Formatting + class declaration cleanup |
| pycopancore/models/_testing/generic_imitation/like_example1.py | Formatting + import normalization |
| pycopancore/model_components/seven_dwarfs/model.py | Rename interface alias + import formatting |
| pycopancore/model_components/seven_dwarfs/interface.py | Formatting + remove unused import |
| pycopancore/model_components/seven_dwarfs/implementation/world.py | Formatting cleanup |
| pycopancore/model_components/seven_dwarfs/implementation/social_system.py | Formatting + cleanup unused imports |
| pycopancore/model_components/seven_dwarfs/implementation/individual.py | Formatting + cleanup unused imports |
| pycopancore/model_components/seven_dwarfs/implementation/group.py | Formatting + rename interface alias |
| pycopancore/model_components/seven_dwarfs/implementation/culture.py | Formatting cleanup |
| pycopancore/model_components/seven_dwarfs/implementation/cell.py | Formatting + cleanup unused imports |
| pycopancore/model_components/seven_dwarfs/implementation/init.py | Export ordering + add __all__ |
| pycopancore/model_components/seven_dwarfs/init.py | Formatting + add __all__ |
| pycopancore/model_components/base/model.py | Import normalization + interface aliasing |
| pycopancore/model_components/base/implementation/world.py | Refactor imports + process list change |
| pycopancore/model_components/base/implementation/social_system.py | Formatting + refactor variable names |
| pycopancore/model_components/base/implementation/metabolism.py | Formatting + interface aliasing |
| pycopancore/model_components/base/implementation/individual.py | Add network registration handling + formatting |
| pycopancore/model_components/base/implementation/group.py | Add network registration handling + formatting |
| pycopancore/model_components/base/implementation/environment.py | Formatting + import normalization |
| pycopancore/model_components/base/implementation/culture.py | Formatting + import normalization |
| pycopancore/model_components/base/implementation/cell.py | Remove/alter social_systems caching logic |
| pycopancore/model_components/base/implementation/init.py | Export ordering + add __all__ |
| pycopancore/model_components/base/init.py | Wildcard export + add __all__ |
| pycopancore/model_components/abstract/world.py | Formatting + import normalization |
| pycopancore/model_components/abstract/social_system.py | Formatting + import normalization |
| pycopancore/model_components/abstract/model.py | Formatting cleanup |
| pycopancore/model_components/abstract/metabolism.py | Formatting + import normalization |
| pycopancore/model_components/abstract/individual.py | Formatting + import normalization |
| pycopancore/model_components/abstract/group.py | Formatting + import normalization |
| pycopancore/model_components/abstract/environment.py | Formatting + import normalization |
| pycopancore/model_components/abstract/culture.py | Formatting + import normalization |
| pycopancore/model_components/abstract/cell.py | Formatting + import normalization |
| pycopancore/model_components/abstract/init.py | Export ordering + add __all__ |
| pycopancore/data_model/unit.py | Formatting + import normalization |
| pycopancore/data_model/set_variable.py | Formatting + docstring wrapping |
| pycopancore/data_model/reference_variable.py | Formatting + docstring wrapping |
| pycopancore/data_model/ordered_set.py | Exception handling cleanup + formatting |
| pycopancore/data_model/master_data_model/world.py | Formatting + import normalization |
| pycopancore/data_model/master_data_model/social_system.py | Formatting + wrap Variable declarations |
| pycopancore/data_model/master_data_model/individual.py | Formatting cleanup |
| pycopancore/data_model/master_data_model/group.py | Formatting + comment cleanup |
| pycopancore/data_model/master_data_model/dimensions_and_units.py | Formatting + wrap long docstrings |
| pycopancore/data_model/master_data_model/culture.py | Formatting + wrap long Variable docs |
| pycopancore/data_model/master_data_model/cell.py | Formatting + import normalization |
| pycopancore/data_model/master_data_model/init.py | Rework exports of units/dimensions |
| pycopancore/data_model/dimensional_quantity.py | Formatting + safer asserts |
| pycopancore/data_model/dimension.py | Formatting + wrap long expressions |
| pycopancore/data_model/init.py | Reformat module docstring + adjust exports |
| pycopancore/init.py | Change fallback version + export base classes |
| docs/introduction.rst | Update install/testing instructions |
| README.md | Expand documentation + add badges/sections |
| CONTRIBUTING.md | Rewrite contributor/release instructions |
| CITATION.cff | Update version and release date |
| .readthedocs.yml | Add Read the Docs configuration |
| .gitignore | Ignore generated _version.py |
| .github/workflows/release.yml | Add release workflow (build/test/lint/publish) |
| .github/workflows/check.yml | Enable tests/black/flake8 in CI |
| .flake8 | Add flake8 configuration file |
Comments suppressed due to low confidence (5)
pycopancore/init.py:21
pycopancore.__init__no longer exposesmaster_data_model, but repository scripts/templates import it viafrom pycopancore import master_data_model as D(e.g. studies/esd_description_paper_example/run_esd_example.py). Re-exportpycopancore.data_model.master_data_model(or provide apycopancore/master_data_model.pyshim) to avoid breaking those entry points.
from pycopancore.model_components.base import (
Individual,
Group,
SocialSystem,
Cell,
World,
Environment,
Metabolism,
Culture,
)
templates/model_components/SOME_COMPONENT/implementation/cell.py:56
- This comment appears to contain commented-out code.
templates/model_components/SOME_COMPONENT/implementation/individual.py:57 - This comment appears to contain commented-out code.
templates/model_components/SOME_COMPONENT/implementation/social_system.py:57 - This comment appears to contain commented-out code.
templates/model_components/SOME_COMPONENT/implementation/world.py:57 - This comment appears to contain commented-out code.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+63
to
+66
| [tool.black] | ||
| line-length = 79 | ||
| target-version = ['py37', 'py38', 'py39', 'py310'] | ||
| exclude = "pycopancore/_version.py" |
There was a problem hiding this comment.
[tool.black].target-version includes py37/py38 even though requires-python is >=3.9. This can unnecessarily restrict formatting decisions and is confusing for contributors; consider setting it to only the supported versions (e.g. ['py39', 'py310', 'py311', 'py312']).
Comment on lines
+60
to
+73
| from pycopancore.data_model import Unit, Dimension | ||
|
|
||
| dim = Dimension("length") | ||
| return Unit("m", "meter", dim) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_dimensional_quantity(): | ||
| """Create a sample DimensionalQuantity instance for testing.""" | ||
| from pycopancore.data_model import DimensionalQuantity, Unit, Dimension | ||
|
|
||
| dim = Dimension("length") | ||
| unit = Unit("m", "meter", dim) | ||
| return DimensionalQuantity(5.0, unit) |
There was a problem hiding this comment.
Unit.__init__ makes parameters after desc keyword-only (* in signature), so Unit("m", "meter", dim) will raise TypeError. Pass dimension=dim (and similarly update the unit construction in sample_dimensional_quantity).
Comment on lines
237
to
246
| @groups.setter | ||
| def groups(self, g): | ||
| """Set the World the SocialSystem is part of.""" | ||
| if self._groups is not None: | ||
| self._groups._social_system.remove(self) | ||
| assert isinstance(g, I.Culture), "groups must be of taxon type Group" | ||
| assert isinstance( | ||
| g, Interface.Culture | ||
| ), "groups must be of taxon type Group" | ||
| g._worlds.add(self) | ||
| self._culture = g |
There was a problem hiding this comment.
SocialSystem.groups setter is internally inconsistent: it asserts g is a Culture, mutates g._worlds, and assigns self._culture, even though this is the setter for groups. This will corrupt state when used. The setter should accept a collection of Group instances (or be removed if groups are managed elsewhere), and it must not write to _culture/_worlds.
| if: steps.version.outputs.should_release == 'false' | ||
| run: | | ||
| echo "Skipping release - no version tag found" | ||
| exit 0 |
There was a problem hiding this comment.
The release workflow doesn’t actually stop when no version tag is detected: should_release is only ever set to false and the “Check if release should proceed” step exits 0 without preventing later steps (so the job continues and can attempt a PyPI upload with empty/incorrect outputs). Make the entire job (or all subsequent steps) conditional on a positive signal (e.g., trigger on push tags v*, or set should_release=true on success and add if: steps.version.outputs.should_release == 'true' to the rest / use a job-level if).
Suggested change
| exit 0 | |
| exit 1 |
| title: 'pycopancore: Reference implementation of the copan:CORE World-Earth modelling framework' | ||
| version: 0.8.6 | ||
| date-released: '2025-09-22' | ||
| version: 0.9.1 |
There was a problem hiding this comment.
Package version information is inconsistent across the repo: CITATION.cff is bumped to 0.9.1, but pycopancore.__init__ falls back to 0.8.3 and pyproject.toml’s hatch fallback_version is also 0.8.3. Align these so that a source checkout without VCS metadata reports the same version that you intend to release/cite.
Suggested change
| version: 0.9.1 | |
| version: 0.8.3 |
| sw.Model, | ||
| base.Model): | ||
|
|
||
| class Model(sd.Model, sw.Model, base.Model): |
There was a problem hiding this comment.
| sw.Model, | ||
| base.Model): | ||
|
|
||
| class Model(sd.Model, sw.Model, base.Model): |
There was a problem hiding this comment.
This class does not call ModelLogics.init during initialization. (The class lacks an init method to ensure every base class init is called.)
| # base[np.where((base == 0)*(exponent < 0))] = 1e-10 | ||
| except: | ||
| # base[np.where((base == 0)*(exponent < 0))] = 1e-10 | ||
| except Exception: |
There was a problem hiding this comment.
This statement is unreachable.
|
|
||
| # load data: | ||
| traj = load(open(filename,"rb")) | ||
| traj = load(open(filename, "rb")) |
There was a problem hiding this comment.
File is opened but is not closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.