Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

preserve Parameter subclass when copying Parameters #964

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

imnotgonnasithereandtryallday

Description

If an instance of a type derived from Parameter is added to a Parameters instance,
and the Parameters instance is then copied, the copy contains a parameter of type Parameter.
This PR changes the contained parameter type to the derived one.

I moved the code copying Parameter instances to Parameter.__deepcopy__ so that the derived class can adds attributes.
This should not break any existing code unless someone uses __deepcopy__ for something else than copying.

Type of Changes
  • Bug fix
Tested on

Python: 3.9.13
lmfit: 0.0.post2850+gb250ed7.d20240807,
scipy: 1.13.1,
numpy: 2.0.0,
asteval: 1.0.2,
uncertainties: 3.2.2

Verification
  • included docstrings that follow PEP 257?
  • referenced existing Issue and/or provided relevant link to mailing list?
    No, I have not opened nor found a relevant Issue.
  • verified that existing tests pass locally?
    Tests do not pass, but the result is the same as on the master branch (the added test passes).
    FAILED tests/test_parameters.py::test_check_ast_errors - Failed: DID NOT RAISE <class 'NameError'>
    1 failed, 650 passed, 12 skipped, 6 warnings in 127.66s (0:02:07)
  • verified that the documentation builds locally?
    Does not build:
    Extension error:
    Could not import extension sphinx.builders.epub3 (exception: cannot import name 'jsonimpl' from partially initialized
    module 'sphinxcontrib.serializinghtml' (most likely due to a circular import)
  • squashed/minimized your commits and written descriptive commit messages?
  • added or updated existing tests to cover the changes?

@newville
Copy link
Member

newville commented Aug 8, 2024

@imnotgonnasithereandtryallday what problem does this solve?

Do we say somewhere that we support using objects derived from Parameter? Parameter has a generic user_data attribute for adding data. Is there a compelling use case for supporting more than that?

I have not looked at the test failures. But version 1.3.2 does pass all tests.

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.

2 participants