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

DownslopeFlowpathLength: --weights accept both raster and constant value #310

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jfbourdon
Copy link
Contributor

Adds a --weight_val switch to allow input of weights as a single value instead of having to use a raster file. The --weights switch has been renamed --weights_file.

I initially wanted to keep the original --weights switch and allow it to accept both a raster file path and a number, but realised that the structure of the tool's parameters doesn't allow it (cleanly), that's my understanding at least. So I instead opt to add a new switch and rename the original one to avoid confusion between the two.

I haven't touch the python API yet, but will do when I'll get feedback on this proposal.

@brownag
Copy link
Contributor

brownag commented Jan 9, 2023

I think this would be a useful feature. But I my 2cents is that it would be best to find a way for --weights to accept both input types or at least leave the existing switch name unchanged for backward compatibility of scripts and various frontends.

@jfbourdon
Copy link
Contributor Author

I agree that it would be the best thing from a user perspective, but doing so isn't very clean in the code so I don't know what @jblindsay would think of that.

The line 79 doesn't do any check so you can already pass a number as parameter:

parameters.push(ToolParameter {
name: "Input Weights File (optional)".to_owned(),
flags: vec!["--weights".to_owned()],
description: "Optional input weights raster file.".to_owned(),
parameter_type: ParameterType::ExistingFile(ParameterFileType::Raster),
default_value: None,
optional: true,
});

The logic would just need to be changed a bit here to check whether the parameter passed is a file (check if the file exists) or is a number (check if it converts f32):

if !weights_file.is_empty() {
use_weights = true;
if !weights_file.contains(&sep) {
weights_file = format!("{}{}", working_directory, weights_file);
}

@jfbourdon
Copy link
Contributor Author

I just found that the parameter type ExistingFileOrFloat exists (used for example in the InPlaceAdd tool). It's much cleaner that way and it avoids breaking backward compatibility.

@jfbourdon jfbourdon changed the title DownslopeFlowpathLength: adds --weight_val switch for input weights as a single value DownslopeFlowpathLength: --weights accept both raster and constant value Feb 2, 2023
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.

2 participants