-
Notifications
You must be signed in to change notification settings - Fork 8
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
Miscellaneous updates to CLI #279
Comments
@nialov any ideas or opinions on these? |
Personally I would keep all the similar command-line functions in
I think how each function fits into the categories will not be obvious to just any user so they might have trouble finding a certain function if they do not know what category it fits into. Difficult to say personally if this would make things clearer or not.
By this you mean the
You can do arbitrary number of values with @app.command()
def hey_there(
random_words: Annotated[List[str], typer.Option()],
):
print(len(random_words))
for word in random_words:
print(word) python3 -m eis_toolkit hey-there --random-words=hey --random-words=there It is a bit cumbersome with adding the option before each value but when done from the plugin it is obviously automated.
I think the progress prints ( with rasterio.open(input_raster) as raster:
data = raster.read() # NOTE: Overlays take in data while for example transforms rasters, consistentency?
# typer.echo("Progress: 25%")
typer.echo(f"Read raster from {input_raster}")
out_image = and_overlay(data)
out_meta = raster.meta.copy()
out_meta["count"] = 1
typer.echo("'And' overlay completed")
typer.echo("Writing raster to {output_raster}") Providing updates in finer increments requires there to be a known number of steps at the for i in range(0, len(bands)):
logging.info(f"Processing band {i+1} out of {len(bands)}")
band_array = raster.read(bands[i]) This kind of logging is quite valuable for debugging when inevitably there will be bugs to hunt. |
Thanks for the comments!
👍 Yes, I like the idea of
Do you think we should also make some changes to the categories in EIS Toolkit? People using the toolkit in their scripts and programs will need to find the tools under some categories anyway, and plugin users will need to find the processing algorithms under some categories too. Somewhat related to this, do you agree with the proposal in issue #239 ?
I agree with you that the it is the best way to document the CLI for people using it directly. I was just pondering if it makes sense to write all the help texts and keep them up-to-date as there will be changes to toolkit function parameters. But this is indeed something that can be decided and added later.
Didn't know this, thanks! This will do the trick for us, as plugin usage is the main focus.
I agree that they do not look pretty in the code, but some algorithms might take long time to execute in the plugin, and if users don't get any updates (and the progress bar stays at 0% all the time), they might think nothing is happening. Maybe the descriptive status updates could work too, or we could leave all progress communication out even if I don't like that idea. One idea I had in mind was to add progress logging into all existing toolkit functions. There could be an environment variable that controls whether these progress updates are printed (and the env variable would be set on in the beginning of CLI functions). However, this would be a fair amount of work and would clutter the toolkit code. |
My reasoning in keeping things in
You have a good point that they will be anyway categorized using the directory names elsewhere. But for the command-line, if you put a function behind a But as I said: Difficult to say personally if this would make things clearer or not. There are pros and cons. If the number of functions grows large, which it might, then I suppose it will be clearer with the groups.
Properly done logging code can replace commenting in code as you can, instead of writing comments, write in the log message the important details happening in the code. I am going to wager a guess that a lot of debugging will happen at some point (hopefully) for which this kind of logging is invaluable to quickly find e.g. logical errors in code. As a side note on the principle of minimal code comments that has been to some extent enforced in |
I have been thinking about a few updates to the CLI:
raster_processing
,prediction
etc. Thecli.py
is getting very large and this could improve maintainability.single_ilr_transform
(takes in two lists of column names). What comes to my mind is that lists could be delivered as one string with a defined separator.The text was updated successfully, but these errors were encountered: