Skip to content

Rename PriorAsProposal to DynamicsProposal, and KFasProposal to KalmanProposal#1131

Merged
sdhiscocks merged 2 commits intodstl:mainfrom
sglvladi:rename-pf-proposals
Mar 4, 2025
Merged

Rename PriorAsProposal to DynamicsProposal, and KFasProposal to KalmanProposal#1131
sdhiscocks merged 2 commits intodstl:mainfrom
sglvladi:rename-pf-proposals

Conversation

@sglvladi
Copy link
Copy Markdown
Collaborator

This PR renames the proposals introduced in #1080 to make them more intuitive and representative of their purpose. Specifically:

  • PriorAsProposal has been renamed to DynamicsProposal
  • KFasProposal has been renamed to KalmanProposal.

This is also an attempt to make the names of the classes consistent with the names that will be used in the paper we aim to publish to Fusion 2025.

Note: This PR introduces breaking changes for code that references the proposals directly. Any legacy code, or more generally code that does not explicitly specify a proposal, should run without errors.

@sglvladi sglvladi requested a review from a team as a code owner February 27, 2025 12:59
@sglvladi sglvladi requested review from orosoman-dstl and spike-dstl and removed request for a team February 27, 2025 12:59
@sglvladi sglvladi force-pushed the rename-pf-proposals branch from e7ceb1f to 4ac7654 Compare February 27, 2025 13:03
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.72%. Comparing base (9393fe7) to head (3dc4602).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
stonesoup/proposal/simple.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1131      +/-   ##
==========================================
- Coverage   93.72%   93.72%   -0.01%     
==========================================
  Files         217      217              
  Lines       14430    14441      +11     
  Branches     1963     1965       +2     
==========================================
+ Hits        13525    13535      +10     
  Misses        644      644              
- Partials      261      262       +1     
Flag Coverage Δ
integration 65.61% <73.33%> (+0.04%) ⬆️
unittests 91.00% <93.33%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sglvladi
Copy link
Copy Markdown
Collaborator Author

Would it be worth to add backwards compatibility with a deprecation warning message for uses of PriorAsProposal and KFasProposal as per PEP562? For example by adding the code below to proposal/simple.py

import warnings

__all__ = ["DynamicsProposal", "KalmanProposal"]

DEPRECATED_NAMES = [("PriorAsProposal", "DynamicsProposal"), ("KFasProposal", "KalmanProposal")]

def __getattr__(name):
    for old_name, new_name in DEPRECATED_NAMES:
        if name == old_name:
            warnings.warn(f"The '{old_name}' class or function has been renamed to '{new_name}' and will be removed in a future release.",
                          DeprecationWarning,
                          stacklevel=2)
            return globals()[new_name]
    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")


def __dir__():
    return sorted(__all__ + [names[0] for names in DEPRECATED_NAMES])

Doing so would allow use of the deprecated class names, but will raise a DeprecationWarning message, e.g.:

>>> from stonesoup.proposal.simple import PriorAsProposal

<input>:1: DeprecationWarning: The 'PriorAsProposal' class or function has been renamed to 'DynamicsProposal' and will be removed in a future release.

>>> print(a := PriorAsProposal(None))
DynamicsProposal(
    transition_model=None)

@sdhiscocks sdhiscocks added the breaking A breaking change that required other to modify their code label Feb 28, 2025
@sdhiscocks
Copy link
Copy Markdown
Member

sdhiscocks commented Feb 28, 2025

Would it be worth to add backwards compatibility with a deprecation warning message for uses of PriorAsProposal and KFasProposal as per PEP562?

Yeah, I think that'd be a good idea. And we'll remove it in the following release afterwards.

@sdhiscocks sdhiscocks merged commit 1dcd56f into dstl:main Mar 4, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking A breaking change that required other to modify their code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants