-
Notifications
You must be signed in to change notification settings - Fork 22
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
overwrite issues with desisim/master/bin/wrap-fastframe #493
Comments
Ok, I think these changes are done: i) add args.clobber to generation of simspec list. Largely goes through fine. Output is here:
Not sure how worrying should be. My problem is that I already have a fork of the desisim repo. with changes to master for BEAST calculations. So I can't submit a pull request, given I can't change the remote directory for this new clone of desisim to point to my github. Any suggestions? The code itself is here: |
caveat: --clobber is passed to fastframe by the wrapper, which doesn't parse --clobber as an argument. Ultimately, desispec.io.write_frame assumes overwrite=True, and uses os.rename to move a .tmp to the destination, both irrespective of args.clobber. |
Short version: thanks, I'll incorporate your changes into my sv branch in PR #494 . Long version: Lesson learned on always working in a branch instead of directly in master. From the DESI wiki UsingGit page: If you made changes to master and you want to retroactively turn that into a branch for further development and/or sending a pull request
IERS warnings are harmless, having to do with not being able to predict the Earths axis wobble (nutation) years into the future. |
ok, great. Looks like it will work. Note the additional clobber issue for fastframe. It wasn't totally obvious what'd you want in that case. |
I think what you did with wrap-fastframe clobber is correct: since fastframe itself will clobber if called, its up to wrap-fastframe whether to call it, based upon --clobber and the pre-existence of files (if not clobbering). Thanks for this. |
Need to port the "if not done, do" logic to the output dir calls. I have the fix. Any objection to providing expose types as an argument? Defaulting to flat,science. |
Lines 83/84 ignore the relevance of args.clobber. If "to clobber", all simspec files should be appended, not just those that do not exist.
Similarly, logfiles should be placed in the dir. referred to by --outdir rather than the dir. of original simspec. Otherwise, e.g. there can be file permission issues when running on a preproduced set of simspec files.
The text was updated successfully, but these errors were encountered: