Skip to content

Conversation

@JR-1991
Copy link
Member

@JR-1991 JR-1991 commented Sep 22, 2025

This pull request addresses an issue raised in #64, where adding compound objects to a class and subsequently updating the dataset fails because the updater is not informed about the changes. This occurs because the updater checks the _changed set to determine if an attribute or collection has changed and should be included in the dataset update. However, when using dedicated add_xyz methods, these fields remain unchanged, leading the updater to assume they should not be updated.

To resolve this issue, the add-function template has been modified to extend the _changed field in both the parent and to-be-added child objects. Additionally, the test suite has been updated to ensure that the system behaves as expected in these scenarios.

Model change tracking and consistency:

  • Updated all references to model_fields in DataverseBase methods to use the class-level attribute (self.__class__.model_fields), ensuring consistent behavior when dealing with inherited fields. [1] [2] [3] [4]

Dynamic function generation and change tracking:

  • Modified generate_add_function in easyDataverse/classgen.py so that added objects are properly tracked as changed, and their own changed fields are updated when created.
  • Improved create_function_signature to skip the _changed attribute when generating function signatures, preventing unintended exposure of internal attributes.

Testing and coverage improvements:

  • Added a new integration test test_dataset_update_with_multiple_fields to verify that multiple fields, including nested and compound fields, are correctly updated and persisted in datasets.
  • Updated unit tests to include the _changed attribute in test model classes and their annotations, ensuring that change tracking works as expected during testing. [1] [2]
  • Added missing imports for Set and PrivateAttr in unit tests to support the new change tracking mechanism.

Issues to be resolved

Added entries for files generated by tests to .gitignore to prevent them from being tracked in version control.
Replaced instance-level access to model_fields with class-level access (self.__class__.model_fields) throughout DataverseBase. This ensures correct field resolution, especially when model_fields is defined at the class level.
The add function now marks the attribute as changed and updates the _changed set of the created subclass object with the provided keyword arguments. Also, _changed is excluded from the generated function signature to prevent unintended parameter exposure.
Introduces a new integration test to verify updating a dataset with multiple fields, including authors, description, contact, and other IDs. Also updates unit tests to include PrivateAttr _changed in test models for change tracking.
@JR-1991 JR-1991 added this to the v0.4.5 milestone Sep 22, 2025
@JR-1991 JR-1991 requested a review from Copilot September 22, 2025 18:37
@JR-1991 JR-1991 self-assigned this Sep 22, 2025
@JR-1991 JR-1991 added the bug Something isn't working label Sep 22, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request fixes an issue where dataset updates fail after adding compound objects using dedicated add_xyz methods. The fix ensures that the updater's change tracking mechanism properly recognizes these additions by updating the _changed set for both parent and child objects.

  • Modified the add-function template to mark attributes as changed when objects are added
  • Updated model field references to use class-level attributes for consistency
  • Added comprehensive integration tests to verify the fix works correctly

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

File Description
easyDataverse/classgen.py Updated add function generation to track changes and skip _changed in signatures
easyDataverse/base.py Changed model field references to use class-level attributes consistently
tests/unit/test_connect.py Added _changed attribute to test models and updated imports
tests/integration/test_dataset_update.py Added integration test for multiple field updates including compound objects

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@JulianRein
Copy link

JulianRein commented Sep 23, 2025

Thank you very much for the quick resolution! Tested it and the .update() works now (after removing and re-installing the package. Maybe bump the version number a little?).
I don't have any rights to merge it obviously, but looks good to me.

Updated the project version from 0.4.4 to 0.4.5 to prepare for a new release.
@JR-1991
Copy link
Member Author

JR-1991 commented Sep 23, 2025

@JulianRein thanks for the quick feedback and the reminder! Bumped the version and will merge the PR now

@JR-1991 JR-1991 merged commit d89c180 into main Sep 23, 2025
3 checks passed
@github-project-automation github-project-automation bot moved this from Ready for Review to Done in PyDataverse Working Group Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Development

Successfully merging this pull request may close these issues.

Changed nested fields fail to upload to dataverse-server ( dataset.update() )

3 participants