jcmgray / xyzpy

Efficiently generate and analyse high dimensional data.

http://xyzpy.readthedocs.io

MIT License

66 stars 11 forks source link

make gen_qsub_script give crop a full absolute parent_dir #9

Closed adamcallison closed 4 years ago

adamcallison commented 4 years ago

Simplest patch I could think of for making qsub_grow work if the Crop's parent_dir is a relative path. Alternative approach would be to modify parse_crop_details to find the absolute path so that Crops always have absolute paths. Thoughts?

jcmgray commented 4 years ago

This looks like it makes sense. Is it necessary to take the cd {working_directory} out? I'm only thinking that maybe the output of the job will no longer be in the working_directory as expected.

codecov-io commented 4 years ago

Codecov Report

Merging #9 into develop will increase coverage by <.01%. The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop       #9      +/-   ##
===========================================
+ Coverage     86.6%   86.61%   +<.01%     
===========================================
  Files           15       15              
  Lines         2875     2877       +2     
===========================================
+ Hits          2490     2492       +2     
  Misses         385      385

Impacted Files	Coverage Δ
xyzpy/gen/batch.py	`91.93% <100%> (+0.03%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ed3035c...37bcd08. Read the comment docs.

adamcallison commented 4 years ago

This looks like it makes sense. Is it necessary to take the cd {working_directory} out? I'm only thinking that maybe the output of the job will no longer be in the working_directory as expected.

I took it out because I don't think it was doing anything - I'm not sure how things work with SGE but on PBS the output files go to where qsub was called. Perhaps the right thing to do is to inherit the working directory from the python process that calls gen_qsub_script, rather than go the crop.parent_dir so that relative paths in the user's actual function get appropriately respected?

jcmgray commented 4 years ago

I think best to leave the cd in, if the function creates log or temporary files or whatever then probably best to have them output in the crop parents directory.

The best policy for dealing with relative paths might be to call os.path.abspath(os.path.expanduser(my_path)), where necessary. What do you think? I definitely haven't thought about the use cases/implementation in a while though...

jcmgray commented 4 years ago

Or I think pathlib is the python 3+ recommended way of doing things these days.

adamcallison commented 4 years ago

Ok, I put the cd back in and switched to pathlib (I hadn't heard of pathlib before - it does seem like a better way)

jcmgray commented 4 years ago

Nice, in the absence of being able to do tests - have you tried it on cx1 with a Crop in a different place / relative parent folder?

adamcallison commented 4 years ago

Yes, just did a simple test case. It works.

jcmgray commented 4 years ago

Amazing, thanks very much again!