From: Thomas Mansencal Date: Sun, 2 Aug 2015 05:55:09 +0000 (+1200) Subject: Implement code review notes. X-Git-Url: http://users.mur.at/ms/git/gitweb/?p=OpenColorIO-Configs.git;a=commitdiff_plain;h=465abef462555baa505ed88b9e6e3f22b4a9c090 Implement code review notes. --- diff --git a/aces_1.0.0/python/aces_ocio/__init__.py b/aces_1.0.0/python/aces_ocio/__init__.py index 5dbea1c..ed02d6b 100644 --- a/aces_1.0.0/python/aces_ocio/__init__.py +++ b/aces_1.0.0/python/aces_ocio/__init__.py @@ -11,7 +11,7 @@ Usage Python ****** ->>> from aces_ocio.config import generate_config +>>> from aces_ocio.generate_config import generate_config >>> aces_ctl_directory = '/path/to/github/checkout/releases/v1.0.0/transforms/ctl' >>> config_directory = '/path/to/configuration/dir' >>> generate_config(aces_ctl_directory, config_directory, 1024, 33, True) diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/aces.py b/aces_1.0.0/python/aces_ocio/colorspaces/aces.py index 33627b8..4f8839d 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/aces.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/aces.py @@ -16,7 +16,7 @@ import shutil import PyOpenColorIO as ocio -from aces_ocio.lut import ( +from aces_ocio.generate_lut import ( generate_1d_LUT_from_CTL, generate_3d_LUT_from_CTL, write_SPI_1d) @@ -737,8 +737,7 @@ def create_ACES_LMT(lmt_name, ctls = [os.path.join(aces_ctl_directory, lmt_values['transformCTLInverse']), shaper_from_aces_ctl % aces_ctl_directory] - # TODO: Investigate unresolved `odt_name` reference. - lut = 'Inverse.%s.%s.spi3d' % (odt_name, shaper_name) + lut = 'Inverse.%s.%s.spi3d' % (lmt_name, shaper_name) lut = sanitize(lut) @@ -1200,8 +1199,8 @@ def create_ODTs(aces_ctl_directory, for odt in sorted_odts: (odt_name, odt_values) = odt - # Defining full range transform for *ODTs* that can only generate - # either *legal* or *full* output. + # Defining full range transform for *ODTs* that can generate either + # *legal* or *full* output. # Uncomment these lines and the lower section and # flip the `legalRange` value to 1 to restore the old behavior, diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/arri.py b/aces_1.0.0/python/aces_ocio/colorspaces/arri.py index 10dc35d..58f52cb 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/arri.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/arri.py @@ -13,7 +13,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.utilities import ColorSpace, mat44_from_mat33, sanitize __author__ = 'ACES Developers' diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/canon.py b/aces_1.0.0/python/aces_ocio/colorspaces/canon.py index 505f6c8..64ffb60 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/canon.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/canon.py @@ -12,7 +12,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.utilities import ColorSpace __author__ = 'ACES Developers' diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/general.py b/aces_1.0.0/python/aces_ocio/colorspaces/general.py index 77102a8..cb935f7 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/general.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/general.py @@ -12,7 +12,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.colorspaces import aces from aces_ocio.utilities import ColorSpace, mat44_from_mat33 diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/gopro.py b/aces_1.0.0/python/aces_ocio/colorspaces/gopro.py index 2431ac9..44b5858 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/gopro.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/gopro.py @@ -12,7 +12,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.utilities import (ColorSpace, sanitize) __author__ = 'ACES Developers' diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/panasonic.py b/aces_1.0.0/python/aces_ocio/colorspaces/panasonic.py index 62d7264..2d32131 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/panasonic.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/panasonic.py @@ -11,7 +11,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.utilities import ColorSpace __author__ = 'ACES Developers' diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/red.py b/aces_1.0.0/python/aces_ocio/colorspaces/red.py index 8ea1b0b..0614a7f 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/red.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/red.py @@ -12,7 +12,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.utilities import ColorSpace, mat44_from_mat33 __author__ = 'ACES Developers' diff --git a/aces_1.0.0/python/aces_ocio/colorspaces/sony.py b/aces_1.0.0/python/aces_ocio/colorspaces/sony.py index 881fd23..a19d2e5 100644 --- a/aces_1.0.0/python/aces_ocio/colorspaces/sony.py +++ b/aces_1.0.0/python/aces_ocio/colorspaces/sony.py @@ -12,7 +12,7 @@ import os import PyOpenColorIO as ocio -import aces_ocio.lut as genlut +import aces_ocio.generate_lut as genlut from aces_ocio.utilities import ColorSpace, mat44_from_mat33 __author__ = 'ACES Developers' diff --git a/aces_1.0.0/python/aces_ocio/config.py b/aces_1.0.0/python/aces_ocio/generate_config.py similarity index 98% rename from aces_1.0.0/python/aces_ocio/config.py rename to aces_1.0.0/python/aces_ocio/generate_config.py index dcbf757..94c6811 100755 --- a/aces_1.0.0/python/aces_ocio/config.py +++ b/aces_1.0.0/python/aces_ocio/generate_config.py @@ -419,7 +419,7 @@ def add_look(config, config_data['colorSpaces'].append(colorspace) - print() + print('') def add_looks_to_views(looks, @@ -575,13 +575,20 @@ def create_config(config_data, if aliases: if reference_data.aliases: - # TODO: Explain context for following comment. # Deferring adding alias colorspaces until end, which helps with - # some applications. + # applications listing the colorspaces in the order that they were + # defined in the configuration: alias colorspaces are usually named + # lower case with spaces but normal colorspaces names are longer + # and more verbose, thus it becomes harder for user to visually + # parse the list of colorspaces when there are names such as + # "crv_canonlog" interspersed with names like + # "Input - Canon - Curve - Canon-Log". + # Moving the alias colorspace definitions to the end of the + # configuration avoids the above problem. alias_colorspaces.append( [reference_data, reference_data, reference_data.aliases]) - print() + print('') if look_info: print('Adding looks') @@ -600,7 +607,7 @@ def create_config(config_data, config_data, multiple_displays) - print() + print('') print('Adding regular colorspaces') @@ -649,15 +656,15 @@ def create_config(config_data, if aliases: if colorspace.aliases: - # TODO: Explain context for following comment. # Deferring adding alias colorspaces until end, which helps - # with some applications. + # with applications listing the colorspaces in the order that + # they were defined in the configuration. alias_colorspaces.append( [reference_data, colorspace, colorspace.aliases]) - print() + print('') - print() + print('') # Adding roles early so that alias colorspaces can be created # with roles names before remaining colorspace aliases are added @@ -680,7 +687,7 @@ def create_config(config_data, texture_paint=prefixed_names[ config_data['roles']['texture_paint']]) - # TODO: Should we remove this dead code path? + # TODO: Pending code path reactivation. # Not allowed at the moment as role names can not overlap # with colorspace names. """ @@ -721,7 +728,7 @@ def create_config(config_data, scene_linear=config_data['roles']['scene_linear'], texture_paint=config_data['roles']['texture_paint']) - # TODO: Should we remove this dead code path? + # TODO: Pending code path reactivation. # Not allowed at the moment as role names can not overlap # with colorspace names. """ @@ -746,7 +753,7 @@ def create_config(config_data, config, reference_data, role_colorspace, [role_name], 'Roles') """ - print() + print('') # Adding alias colorspaces at the end as some applications use # colorspaces definitions order of the configuration to order @@ -758,7 +765,7 @@ def create_config(config_data, for reference, colorspace, aliases in alias_colorspaces: add_colorspace_aliases(config, reference, colorspace, aliases) - print() + print('') print('Adding the diplays and views') @@ -890,7 +897,7 @@ def create_config(config_data, config.setActiveDisplays(','.join(sorted(displays))) config.setActiveViews(','.join(views)) - print() + print('') # Ensuring the configuration is valid. config.sanityCheck() diff --git a/aces_1.0.0/python/aces_ocio/lut.py b/aces_1.0.0/python/aces_ocio/generate_lut.py similarity index 99% rename from aces_1.0.0/python/aces_ocio/lut.py rename to aces_1.0.0/python/aces_ocio/generate_lut.py index b0aab12..962881e 100755 --- a/aces_1.0.0/python/aces_ocio/lut.py +++ b/aces_1.0.0/python/aces_ocio/generate_lut.py @@ -829,11 +829,11 @@ def main(): else: print('3D LUT generation options') - print('Lut : %s' % lut) + print('LUT : %s' % lut) print('Format : %s' % format) print('CTLs : %s' % ctls) - print('Lut Res 1d : %s' % lut_resolution_1d) - print('Lut Res 3d : %s' % lut_resolution_3d) + print('LUT Res 1d : %s' % lut_resolution_1d) + print('LUT Res 3d : %s' % lut_resolution_3d) print('Min Value : %s' % min_value) print('Max Value : %s' % max_value) print('Input Scale : %s' % input_scale) diff --git a/aces_1.0.0/python/aces_ocio/tests/tests_aces_config.py b/aces_1.0.0/python/aces_ocio/tests/tests_aces_config.py index a451379..fb7524a 100644 --- a/aces_1.0.0/python/aces_ocio/tests/tests_aces_config.py +++ b/aces_1.0.0/python/aces_ocio/tests/tests_aces_config.py @@ -19,7 +19,7 @@ sys.path.append(os.path.abspath( os.path.join(os.path.dirname(__file__), '..', '..'))) from aces_ocio.utilities import files_walker -from aces_ocio.config import ( +from aces_ocio.generate_config import ( ACES_OCIO_CTL_DIRECTORY_ENVIRON, generate_config) @@ -40,7 +40,6 @@ __all__ = ['REFERENCE_CONFIG_ROOT_DIRECTORY', # tests. REFERENCE_CONFIG_ROOT_DIRECTORY = os.path.abspath( os.path.join(os.path.dirname(__file__), '..', '..', '..')) -REFERENCE_CONFIG_ROOT_DIRECTORY = '/colour-science/colour-ramblings/ocio/aces' HASH_TEST_PATTERNS = ('\.3dl', '\.lut', '\.csp') UNHASHABLE_TEST_PATTERNS = ('\.icc', '\.ocio') diff --git a/aces_1.0.0/python/bin/create_aces_config b/aces_1.0.0/python/bin/create_aces_config index 0500b41..b62606d 100755 --- a/aces_1.0.0/python/bin/create_aces_config +++ b/aces_1.0.0/python/bin/create_aces_config @@ -12,7 +12,7 @@ import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..')) -from aces_ocio.config import main +from aces_ocio.generate_config import main __author__ = 'ACES Developers' __copyright__ = 'Copyright (C) 2014 - 2015 - ACES Developers' diff --git a/aces_1.0.0/python/bin/generate_lut b/aces_1.0.0/python/bin/generate_lut index 52391b6..80a8f0a 100755 --- a/aces_1.0.0/python/bin/generate_lut +++ b/aces_1.0.0/python/bin/generate_lut @@ -12,7 +12,7 @@ import sys sys.path.append(os.path.join(os.path.dirname(__file__), '..')) -from aces_ocio.lut import main +from aces_ocio.generate_lut import main __author__ = 'ACES Developers' __copyright__ = 'Copyright (C) 2014 - 2015 - ACES Developers'