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.

Sign up for GitHub

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

Jump to bottom

[patch] Add warning when the same job is imported multiple times with unpack #1596

Merged
merged 2 commits into from
Aug 12, 2024
Merged

[patch] Add warning when the same job is imported multiple times with unpack #1596

merged 2 commits into from
Aug 12, 2024

Conversation

samwaseda
Copy link
Member

@samwaseda samwaseda commented Aug 12, 2024 •
edited
Loading

In the current format the following code would import the same job test twice.

from pyiron_base import Project


pr = Project("TEST")
job = pr.create_job(ToyJob, "test")
job.run()
pr.pack("archive")
pr = Project("import")
pr.unpack("archive")
pr.unpack("archive")

And with this PR it does not import the same job and issues a warning.

@samwaseda
Copy link
Member Author

I asked review also from @liamhuber because this was also disturbing him before.

@samwaseda samwaseda added code_smell It's working, but something smells off minor add functionality in a backward compatible manner labels Aug 12, 2024
@samwaseda samwaseda changed the title [minor] Add warning when the same job is imported multiple times with unpack [patch] Add warning when the same job is imported multiple times with unpack Aug 12, 2024
@samwaseda samwaseda added patch backward compatible bug fixes and removed minor add functionality in a backward compatible manner labels Aug 12, 2024
@samwaseda samwaseda merged commit e4ee99c into main Aug 12, 2024
35 checks passed
@samwaseda samwaseda deleted the forbid_multiple_entry branch August 12, 2024 15:50
liamhuber
liamhuber approved these changes Aug 12, 2024
View reviewed changes
Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

Post-merge review: A nit to think about changing later, but wanted to stamp my approval. Nicely backwards compatible with the flag for the feature 👍

pyiron_base/database/generic.py
@@ -631,6 +648,9 @@ def add_item_dict(self, par_dict):
"""
if not self._view_mode:
try:
if check_duplicates and self._check_duplidates(par_dict):
warnings.warn(f"Duplicate entry found in database: {par_dict}")
Copy link
Member

Choose a reason for hiding this comment

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

Only nit is that we might want the logger warning instead of warning.warn.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamhuber liamhuber liamhuber approved these changes

@pmrv pmrv pmrv approved these changes

Assignees
No one assigned
Labels
code_smell It's working, but something smells off patch backward compatible bug fixes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@samwaseda @pmrv @liamhuber