Make nuke_temp_dir work #97
Conversation
…file, but it still does not actually nuke
There was a problem hiding this comment.
- Because I am old and boring, I suggest that this variable be called
remove_temp_dir. - Add test for this function, particularly given that changelog said "make it work".
- Documentation request: What is the relationship between
scratch_dirandtemp_dir?
What is the relationship betweenkeep_intermediateandremove_temp_dir?
I think the behavior in this snippet below might be surprising to the user:
if self.keep_intermediate:
self.save_dir = pathlib.Path( self.config.value( 'photometry.phrosty.paths.scratch_dir' ) )
elif not self.keep_intermediate:
self.save_dir = pathlib.Path( self.config.value( 'photometry.phrosty.paths.temp_dir' ) )
I think whether or not I chose to keep something would be separate from where it was kept. (I do understand how the code ended up this way).
|
I did bring up the idea of merging scratch and intermediate in the past--I don't remember the reasoning for not rolling it all into one. @rknop do you remember? |
|
Also... I think writing tests for anything in pipeline.py is a huge task right now--petition to punt this to the next photphest? |
phrosty/pipeline.py
Outdated
| SNLogger.warning( "nuke_temp_dir not implemented" ) | ||
|
|
||
| self.keep_intermediate = self.config.value( 'photometry.phrosty.keep_intermediate' ) | ||
| self.nuke_temp_dir = self.config.value( 'photometry.phrosty.nuke_temp_dir' ) |
There was a problem hiding this comment.
Updated to remove_temp_dir
(ruff or any other linter can't figure out that "photometry.phrosty.nuke_temp_dir" isn't going to be defined, so it didn't warn you.)
phrosty/pipeline.py
Outdated
| SNLogger.info( f"After make_lightcurve, memory usage = \ | ||
| {tracemalloc.get_traced_memory()[1]/(1024**2):.2f} MB" ) | ||
|
|
||
| if self.nuke_temp_dir: |
|
Oops. Yes, missed a push. |
changes/97.feature.rst
Outdated
| @@ -0,0 +1 @@ | |||
| Move nuke_temp_dir kwarg out of Pipeline function and into the phrosty_config.yaml file. Also, make it work instead of leaving it as an empty parameter. | |||
There was a problem hiding this comment.
Mention rename to remove_temp_dir
Make remove_temp_dir work 78ae13d
No description provided.