path-token-fix

For some systems such as lxplus, directories may have a dot in their path. As such, the current code can not save or import the fit properly.

Solution/simple test:

fold_i = 0
path = "/system.directory/path/to/model"
path_token = path.rsplit("/", 1)
file_token = path_token[-1].rsplit(".", 1)

if len(file_token) == 1:
    file_token.append(f"fold_{fold_i}")
else:
    file_token.insert(-1, f"fold_{fold_i}")

if len(path_token) == 1:
    path_token = [".".join(file_token)]
else:
    path_token = [path_token[0]] + [".".join(file_token)]

print("/".join(path_token))

Tests:

path = "/system.directory/path/to/model" -> /system.directory/path/to/model.fold_0

path = "/system.directory/path/to/model.h5" -> /system.directory/path/to/model.fold_0.h5

path = "/system_directory/path/to/model" -> /system_directory/path/to/model.fold_0

path = "/system_directory/path/to/model.h5" -> /system_directory/path/to/model.fold_0.h5

path = "model" -> model.fold_0

path = "model.h5" -> model.fold_0.h5

added Type::Bug label

requested review from @fsauerbu

By Ahmed Markhoos on 2022-09-21T13:51:11 (imported from GitLab)

assigned to @fsauerbu and @ahmarkho

By Ahmed Markhoos on 2022-09-21T13:51:11 (imported from GitLab)

That's great contribution, thank you very much

Looking at the changes, it might be worth to define a helper method _get_fold_path() or something like this. Then, we don't need to implement the same logic twice and it's easy to add the test cases listed above. Also, there might be a much simpler way to implement this with os.path.splitext (I didn't know when I wrote the code original code).

Concerning the failing CI, that's something I can have a look at.

added 1 commit

• 49ce9eb2 - add _get_model_path() and its test function

Compare with previous version

By Ahmed Markhoos on 2022-09-22T11:41:25 (imported from GitLab)

added 1 commit

• 21ea2cbd - Add scipy dependency

Compare with previous version

resolved all threads

marked this merge request as ready

merged