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

spec pipeline file/dir naming cleanup #559

Merged
merged 2 commits into from Mar 28, 2018
Merged

spec pipeline file/dir naming cleanup #559

merged 2 commits into from Mar 28, 2018

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Mar 28, 2018

This PR provides some file and directory naming cleanup:

  • remove colons from spectro pipeline output directory and file names since they are a special character for Unix.
    • old: run/scripts/redshift_20180328-00:28:04/run_20180328-00:28:04.log
    • new: run/scripts/redshift_20180328-002804/run_20180328-002804.log
    • the new scheme is admitted less obviously readable as a time, but easier to work with when cutting and pasting and browsing directories with tab completion.
  • use desispec.io.findfile() to get correct redrock hdf5 output file name

Additional features that came along for the ride while debugging the above:

  • Fixed "fail" vs. "failed" typos in pipeline state tracking
  • Added a --skip-psf option to desispec.test.integration_test for testing without spexec installed

Copy link
Contributor

@julienguy julienguy left a comment

Choose a reason for hiding this comment

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

tested

@julienguy
Copy link
Contributor

Stephen, why did you remove the backward compatibility of PSF I/O in traceshifts.py .
I would rather keep it for now because we have EM spectrograph PSFs in the old format on disk.

@sbailey
Copy link
Contributor Author

sbailey commented Mar 28, 2018

It did not have backwards compatibility (it assumed only X/YTRACE and didn't look for X/YCOEFF). I tried adding it, which sort of worked, but resulted in extractions that has resolution matrices normalized to ~0.5 instead of ~1.0, thus eventually causing redshift fitting to fail due to masking. Rather than go further down that rabbit hole, I backed out my changes so I wouldn't leave messy if/then code that was only a partial solution anyway. i.e. there is a commit message about undoing changes for old PSFs, but in the end this PR doesn't impact any previous PSF format functionality (working or otherwise).

@julienguy
Copy link
Contributor

ok. thanks for the clarification. merging.

@julienguy julienguy merged commit 416c4cc into master Mar 28, 2018
@julienguy julienguy deleted the naming branch March 28, 2018 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants