New syntax for f-strings for Python >= 3.12 #105
Conversation
cosimolupo
changed the title
|
I tested the full pipeline, branch bugfix/fstrings with data and profiles as in https://github.com/APE-group/hands_on_cobrawap/tree/main/test_datasets/imaging_deep_anesthesia. No problems/errors encountered |
| @@ -13,7 +13,7 @@ def input_file(wildcards): | |||
|
|
|||
| def is_clear(wildcards): | |||
| if config.RERUN_MODE: | |||
| return Path(f'{wildcards.dir}') / 'clear.done' | |||
| return Path(str('{wildcards.dir}')) / "clear.done" | |||
There was a problem hiding this comment.
| return Path(str('{wildcards.dir}')) / "clear.done" | |
| return Path(wildcards.dir) / "clear.done" |
wildcards.dir should already be a string. I think it can be simplified like this
| # configfile = Path('configs') / f'config_{PROFILE}.yaml' | ||
|
|
||
| rule clear: | ||
| output: | ||
| temp(Path('{path}') / 'clear.done') | ||
| params: | ||
| block_folder = [Path('{path}') / f'{block}' for block in config.BLOCK_ORDER] | ||
| block_folder = [Path('{path}') / str('{block}') for block in config.BLOCK_ORDER] |
There was a problem hiding this comment.
| block_folder = [Path('{path}') / str('{block}') for block in config.BLOCK_ORDER] | |
| block_folder = [Path('{path}') / block for block in config.BLOCK_ORDER] |
this interprets '{block}' as literal, causing an error because ../path/'{block}' doesn't exist.
| params(channels=config.PLOT_CHANNELS, | ||
| img_name='processed_trace_channel0.'+config.PLOT_FORMAT, | ||
| params(t_start=config.PLOT_TSTART, t_stop=config.PLOT_TSTOP, | ||
| channels=config.PLOT_CHANNELS, | ||
| img_name="processed_trace_channel0." + config.PLOT_FORMAT, | ||
| original_data=config.STAGE_INPUT) | ||
| output: | ||
| img_dir = directory(OUTPUT_DIR / 'processed_traces_{t_start}-{t_stop}s') | ||
| img_dir = directory(OUTPUT_DIR / | ||
| str("processed_traces_%s-%ss" % (config.PLOT_TSTART,config.PLOT_TSTOP))) |
There was a problem hiding this comment.
this edit changes the flexibility of the rule. t_start and t_stop should be determined via the wildcards and not via the config.
The config values are only used in the all rule of this stages that defines str("processed_traces_%s-%ss" % (config.PLOT_TSTART,config.PLOT_TSTOP))) as its input
There was a problem hiding this comment.
Yes, I'm aware of this change in the behavior. I was wondering how to preserve this flexibility by using the new syntax. Any suggestion?
| # 'zscore', 'subsampling' | ||
| # 'z_score', 'subsampling' |
There was a problem hiding this comment.
I agree with this changes, but it breaks backwards compatibility and has to be explicitly marked in release notes!
There was a problem hiding this comment.
Yes, correct. I changed it coherently everywhere, let's mark it explicitly in the release notes.
| params() | ||
| params(t_start=config.SLICE_TSTART,t_stop=config.SLICE_TSTOP) | ||
| output: | ||
| Path('{dir}') / f'{{data_name}}_{{t_start, [0-9]+}}-{{t_stop, [0-9]+}}s.{config.NEO_FORMAT}' | ||
| Path('{dir}') / str('{data_name}' + ("_%s-%ss." % (config.SLICE_TSTART, config.SLICE_TSTOP)) + config.NEO_FORMAT) |
There was a problem hiding this comment.
same issue as in previous comment. The rule should maintain its flexibility by setting the t_start and t_stop values via the wildcards and not the config values
There was a problem hiding this comment.
Like this, you would need to still write the snakemake call with the t_start and t_stop times in the filename, but now it only works if it matches the config settings
There was a problem hiding this comment.
Indeed... as said before, let's find a solution that is coherent with the new syntax, still preserving this degree of flexibility.
| output: | ||
| Path('{path}_{scale}px.gif') | ||
| Path(str('{path}' + "_" + '{scale}' + "px.gif")) |
There was a problem hiding this comment.
| Path(str('{path}' + "_" + '{scale}' + "px.gif")) | |
| Path('{path}' + "_" + '{scale}' + "px.gif") |
|
|
||
| # TIME_SLICE | ||
| # Used for cutting the signal before video production | ||
| ################ | ||
| SLICE_TSTART: 0 # float (in s) or 'None' -> starting time of the input signal is used | ||
| SLICE_TSTOP: 10 # float (in s) or 'None' -> stopping time of the input signal is used |
There was a problem hiding this comment.
remove, see comment above
| return [OUTPUT_DIR / m / str(config.EVENT_NAME + "_" + m + ".csv") | ||
| for m in config.MEASURES] |
There was a problem hiding this comment.
| return [OUTPUT_DIR / m / str(config.EVENT_NAME + "_" + m + ".csv") | |
| for m in config.MEASURES] | |
| return [OUTPUT_DIR / measure / str(config.EVENT_NAME + "_" + measure + ".csv") | |
| for measure in config.MEASURES] |
There was a problem hiding this comment.
I was trying to avoid any potential overlap between actual snakemake wildcards and other dummy variables used for iterations.
| Path('{dir}') / 'spatial_derivative' | ||
| / str(config.EVENT_NAME + "_spatial_derivative.csv"), | ||
| output_img = Path('{dir}') / 'spatial_derivative' | ||
| / str(config.EVENT_NAME + "_spatial_derivative." + config.PLOT_FORMAT) |
There was a problem hiding this comment.
| Path('{dir}') / 'spatial_derivative' | |
| / str(config.EVENT_NAME + "_spatial_derivative.csv"), | |
| output_img = Path('{dir}') / 'spatial_derivative' | |
| / str(config.EVENT_NAME + "_spatial_derivative." + config.PLOT_FORMAT) | |
| Path('{dir}') / '{rule_name}' | |
| / str(config.EVENT_NAME + "_spatial_derivative.csv"), | |
| output_img = Path('{dir}') / '{rule_name}' | |
| / str(config.EVENT_NAME + "_spatial_derivative." + config.PLOT_FORMAT) |
| return [OUTPUT_DIR / m / str(config.EVENT_NAME + "_" + m + ".csv") | ||
| for m in config.MEASURES] |
There was a problem hiding this comment.
| return [OUTPUT_DIR / m / str(config.EVENT_NAME + "_" + m + ".csv") | |
| for m in config.MEASURES] | |
| return [OUTPUT_DIR / measure / str(config.EVENT_NAME + "_" + measure + ".csv") | |
| for measure in config.MEASURES] |
As explained in issue #99, Python version 3.12 has changed f-strings handling, causing current syntax of f-strings in Cobrawap -- especially in Snakefiles -- to not be correctly interpreted and execution to fail. This PR changes such syntax, providing an alternative. Also some regex now need to be handled more carefully.
When Snakemake version will be upgraded, this hack will not be necessary any longer, and in principle the old f-string syntax could be put back in place.