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

[patch] Restructure archive #1595

Merged
merged 20 commits into from
Aug 14, 2024
Merged

[patch] Restructure archive #1595

merged 20 commits into from
Aug 14, 2024

Conversation

samwaseda
Copy link
Member

@samwaseda samwaseda commented Aug 12, 2024

Code snippet

from pyiron_base import Project
from pyiron_base._tests import ToyJob

pr = Project("my/project")
job = pr.create_job(ToyJob, "my_job")
job.run()
sub_pr = pr.create_group("my_group")
job = sub_pr.create_job(ToyJob, "my_sub_job")
job.run()
pr.pack("archive")

pr_import = Project("import")
pr_import.unpack("archive")

Job Table

# All other entries are identical
print(pr_import.job_table()["project"])

Old output:

project
0 /home/jovyan/RESEARCH/2024/0812/PACK/import/project/
1 /home/jovyan/RESEARCH/2024/0812/PACK/import/project/my_group/

New output:

project
0 /home/jovyan/RESEARCH/2024/0812/PACK/import/
1 /home/jovyan/RESEARCH/2024/0812/PACK/import/my_group/

Archive structure

Old archive.tar.gz structure:

archive/
archive/project/
archive/project/my_group/
archive/project/my_group/my_sub_job.h5
archive/project/my_job.h5

New archive.tar.gz structure:

my/project/
my/project/my_group/
my/project/my_group/my_sub_job.h5
my/project/my_job.h5

Potential problem with the new version

When a subgroup is created and the main group has no job, the sub group will be considered as the project path, i.e.:

pr = Project("my_project")
pr_sub = pr.create_group("my_group")
job = pr_sub.create.job.MyJob("my_job")
job.run()

is equivalent to

pr = Project("my_project/my_group")
job = pr.create.job.MyJob("my_job")
job.run()

and in both cases my_project/my_group will be replaced by the new project name. If the main project contains at least one job or other sub groups then only my_project will be replaced and the overall structure will be maintained.

From what I can see the changes are backward compatible. Frankly I cannot really guarantee anything.

@samwaseda samwaseda marked this pull request as draft August 12, 2024 08:13
@samwaseda samwaseda marked this pull request as ready for review August 13, 2024 15:08
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.

I'm not deeply enough into this corner of the code base to make any comment on function, but in form this seems perfectly reasonable to me. "Comment"-status here is meant to indicate my own lack of self-confidence as a reviewer, not as a slam on the PR.

Added a nit you should take, a nit you should probably ignore (cost/benefit is bad), and a cute suggestion.

tests/unit/archiving/test_import.py Outdated Show resolved Hide resolved
@samwaseda
Copy link
Member Author

I'm not deeply enough into this corner of the code base to make any comment on function, but in form this seems perfectly reasonable to me. "Comment"-status here is meant to indicate my own lack of self-confidence as a reviewer, not as a slam on the PR.

It's true that my intention was more like "hey this is what we agreed on yesterday, right?" and not so much about asking you guys to review the code, because except for @pmrv there's no one who has followed the updates so I guess it's difficult for you to understand each line of code.

samwaseda and others added 2 commits August 13, 2024 20:57
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
@samwaseda samwaseda added the patch backward compatible bug fixes label Aug 13, 2024
@github-actions github-actions bot changed the title Restructure archive [patch] Restructure archive Aug 13, 2024
@samwaseda samwaseda merged commit 6658f77 into main Aug 14, 2024
30 checks passed
@samwaseda samwaseda deleted the restructure_archive branch August 14, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch backward compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants