Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Global: colorspace parse_colorspace_from_filepath simplification #4348

Open
jakubjezek001 opened this issue Jan 18, 2023 · 1 comment
Open

Comments

@jakubjezek001
Copy link
Member

jakubjezek001 commented Jan 18, 2023

Would this function make more sense when simplified/rewritten as just finding the first relevant color space from the list of color spaces of the config? It'd make it more versatile in use instead of it internally always needing to call get_ocio_config_colorspaces.

It feels to me that parsing from filenames like this might often be frequently done close after each other - e.g. parsing multiple files. Quering the color space files rules will be relatively slow since it requires accessing the OCIO config file each time.

colorspaces = get_ocio_config_colorspaces(config_path)
for filepath in filepaths:
    colorspace = parse_colorspace_from_filepath(filepath, colorspaces=colorspaces)

This also looks to me like a much more friendly API to the user with much less arguments to pass. Plus it'd be much faster and a simpler function code body.

As a general recommendation I'd try to see if there are other areas where functions could be simplified like this. Needing to pass project name, host, project settings to all functions where they end up just parsing an OCIO config path I'd say should be rewritten to just take the config path.

Originally posted by @BigRoy in #4195 (comment)

[cuID:OP-4770]

@jakubjezek001
Copy link
Member Author

jakubjezek001 commented Jan 19, 2023

This is also need to address following issue from @BigRoy comment here #4195 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant