thingsvision icon indicating copy to clipboard operation
thingsvision copied to clipboard

further changes to the extraction pipeline

Open LukasMut opened this issue 2 years ago • 0 comments

There are a few changes I would like to see, myself, but this should ultimately be Lukas's decision.

I think we should ask for an out-folder and automatically name it with the layer the user is asking for. I find even myself, I've run code over multiple iterations and left the same out-path and it's overwritten features.npy and file_names.txt and I've needed to go back again. I think if we ask for a folder, then save in .../out_folder/module_name/features.npy it will be more useful, adding in a check to make the folder, if necessary, beforehand (Path(..).mkdir(parents=True, exist_ok=True) does this check easily enough). That way you could run the same command line code but only change module-name and it would instantly just put everything in the right place. That would be how I sort of expect this and would be easier for me, as I think a lot of people want features from multiple layers, and it's always easier to forget to make two changes to your calls, when only one is really necessary. When that happens, features.npy just gets overwritten and you have to run the whole thing again.

I also think out-path (or out-folder as I would want to call it) should be echoed back to the user after the extract-features call. Right now it just gives you the shape and a confirmation that the features were saved to disk.

So, only relatively small changes that are desirable but these could wait until the next release. I also wanted to re-echo the idea for an automatic lookup so that a relevant source can be selected based on the model name, so the user doesn't need to specify it. It's on us as people developing the library to work this out for the user (I think). If a researcher wants to use a specific model they've heard about, I think it should be sufficient to just input the model name (but advanced users can always select a specific source they want). Again, this isn't directly related to this CLI PR, but solving that would make the CLI experience a little easier.

All in all, at some point I would like to see a change in out-path if others agree (perhaps a good argument can be made for keeping it as is - that I haven't considered).

Originally posted by @Alxmrphi in https://github.com/ViCCo-Group/thingsvision/pull/104#pullrequestreview-1203696408

LukasMut avatar Dec 06 '22 13:12 LukasMut