-
Notifications
You must be signed in to change notification settings - Fork 15
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
Copy files in copy_to #349
Conversation
Thanks @pmrv for trying to fix this issue. I tried to copy a project with some DFT calcs into another project using this branch. Below is the error I received. Along with the below error, the ase_to_pyiron function is giving me some error when I switched to this branch. I am not sure if it has to do with the changes @pmrv made, but it could be because of some other pull request related to pyiron_atomistics. For now, I think I will stick to my original branch rather than updating my pyiron and breaking my notebooks. Switching to this branch:
Code in notebook:
Error:
|
I'm a bit confused, because this error occurs loading the job before the copying it. Can you check that you can load all the jobs in original (source) project? |
Thanks @pmrv for going through the error. Actually, yes, you are right. There is an issue with loading jobs with this branch. When I try with the convert_to_object=False option, I do not get an error.
However, I get the same error as above when I try to load a job with the default convert_to_object=True option
Error:
PS: With the previous branch (my master) I was using, I did not get this error when loading jobs.
I get this warning, but everything works fine.
|
@sravankumarkb how does your master compare to the master branch? Since this PR does not touch anything on loading of jobs and, thus, I assume the master branch will have the same issue to load your jobs. |
Good, then we're onto something already! ;) Actually, while going through the code earlier I've noticed that if you call j = pr.inspect(...)
j.copy_to(...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am ok with exposing the copy_files
option. I just wonder why the default was False
and if we should change the default here?
@raynol-dsouza does this have any bearing on our trouble loading protocol jobs? It's only been a week but already the details are hazy to me 😉 IIRC part of our child creation process got back a valid job object but the wrong/old HDF location. |
I just think default |
@liamhuber no, loading protocol jobs had to do with the sequence in which the hdf content was being saved/loaded.
the wrong hdf location, yes. But I believe this might totally be a Protocols related issue, and not a |
@sravankumarkb Did you check the loading again with the latest master branch? If it still won't work, I'll have another look. |
Hi @pmrv
I tried loading a pyiron table, but I get the following error.
When I tried loading the pyiron table with convert_to_object=False and copy it, I get the following error.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
What is the current status here? |
It regrettably dropped from my radar. @sravankumarkb The first issue occurs before |
Hi Marvin @pmrv |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@sravankumarkb Can you give it another go with this version? EDIT: Actually hold off until the tests pass. |
ee51a9d
to
03019bd
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Since the last commit already creates the directory (to make sure copying the HDF5 file works), shutil.copytree bailed without the `dirs_exist_ok` flag. If the directory is not empty we raise an error though.
@sravankumarkb This should be good to go now. |
GenericJob
could previously copy files from the working directory, but didn't by default. I exposedcopy_files
as an argument and change the default toTrue
, same as inJobCore
.Fixes #348.