Repository: omni-us/jsonargparse
Pull request #163 (2022-09-27 03:15:04)

Import speed improvements
## What does this PR do?

Implements some changes that speed up import time and avoids importing optional modules which in many use cases are not used.

## Before submitting

- [x] Did you read the [contributing guideline](https://github.com/omni-us/jsonargparse/blob/master/CONTRIBUTING.rst)?
- [n/a] Did you update **the documentation**? (readme and public docstrings)
- [x] Did you write **unit tests** such that there is 100% coverage on related code? (required for bug fixes and new features)
- [x] Did you verify that new and existing **tests pass locally**?
- [x] Did you make sure that all changes preserve **backward compatibility**?
- [x] Did you update **the CHANGELOG**? (not for typos, docs, test updates, or minor internal changes/refactors)


sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=omni-us_jsonargparse&pullRequest=163)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=omni-us_jsonargparse&pullRequest=163&resolved=false&types=CODE_SMELL)

[![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=163) No Coverage information  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=163&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=omni-us_jsonargparse&pullRequest=163&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 913e650..f2bce56 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -14,6 +14,16 @@ The semantic versioning only considers the public API as described in
 paths are considered internals and can change in any moment.
 
 
+v4.15.0 (2022-09-??)
+--------------------
+
+Changed
+^^^^^^^
+- Now ``UUID`` and ``timedelta`` types are registered on first use to avoid
+  possibly unused import.
+- json/yaml dump sort now defaults to false for all python implementations.
+
+
 v4.14.1 (2022-09-26)
 --------------------
 
diff --git a/jsonargparse/loaders_dumpers.py b/jsonargparse/loaders_dumpers.py
index 2c23146..4772807 100644
--- a/jsonargparse/loaders_dumpers.py
+++ b/jsonargparse/loaders_dumpers.py
@@ -8,7 +8,7 @@ from contextlib import contextmanager
 from contextvars import ContextVar
 from typing import Any, Callable, Dict, Tuple, Type
 
-from .optionals import dump_preserve_order_support, import_jsonnet
+from .optionals import import_jsonnet
 from .type_checking import ArgumentParser
 
 
@@ -118,12 +118,12 @@ def load_value(value: str, **kwargs):
 dump_yaml_kwargs = {
     'default_flow_style': False,
     'allow_unicode': True,
-    'sort_keys': False if dump_preserve_order_support else True,
+    'sort_keys': False,
 }
 
 dump_json_kwargs = {
     'ensure_ascii': False,
-    'sort_keys': False if dump_preserve_order_support else True,
+    'sort_keys': False,
 }
 
 
diff --git a/jsonargparse/optionals.py b/jsonargparse/optionals.py
index fd9d4b7..f9845d5 100644
--- a/jsonargparse/optionals.py
+++ b/jsonargparse/optionals.py
@@ -3,7 +3,6 @@
 import inspect
 import locale
 import os
-import platform
 import typing
 from contextlib import contextmanager
 from importlib.util import find_spec
@@ -27,7 +26,6 @@ fsspec_support = find_spec('fsspec') is not None
 ruyaml_support = find_spec('ruyaml') is not None
 omegaconf_support = find_spec('omegaconf') is not None
 reconplogger_support = find_spec('reconplogger') is not None
-dump_preserve_order_support = platform.python_implementation() == 'CPython'
 
 _config_read_mode = 'fr'
 _docstring_parse_options = {
diff --git a/jsonargparse/typehints.py b/jsonargparse/typehints.py
index 1ba0445..9a865e0 100644
--- a/jsonargparse/typehints.py
+++ b/jsonargparse/typehints.py
@@ -34,7 +34,7 @@ from typing import (
 from .actions import _ActionHelpClassPath, _find_action, _find_parent_action, _is_action_value_list
 from .loaders_dumpers import get_loader_exceptions, load_value, load_value_context
 from .namespace import Namespace
-from .typing import is_final_class, registered_types
+from .typing import get_registered_type, is_final_class
 from .optionals import (
     argcomplete_warn_redraw_prompt,
     get_files_completer,
@@ -171,7 +171,7 @@ class ActionTypeHint(Action):
         supported = \
             typehint in root_types or \
             get_typehint_origin(typehint) in root_types or \
-            typehint in registered_types or \
+            get_registered_type(typehint) or \
             is_subclass(typehint, Enum) or \
             ActionTypeHint.is_subclass_typehint(typehint)
         if full and supported:
@@ -200,7 +200,7 @@ class ActionTypeHint(Action):
             return test(ActionTypeHint.is_subclass_typehint(s) for s in subtypes)
         return inspect.isclass(typehint) and \
             typehint not in leaf_or_root_types and \
-            typehint not in registered_types and \
+            not get_registered_type(typehint) and \
             typehint_origin is None and \
             not is_subclass(typehint, (Path, Enum))
 
@@ -466,7 +466,7 @@ def adapt_typehints(val, typehint, serialize=False, instantiate_classes=False, p
     # Any
     if typehint == Any:
         type_val = type(val)
-        if type_val in registered_types or is_subclass(type_val, Enum):
+        if get_registered_type(type_val) or is_subclass(type_val, Enum):
             val = adapt_typehints(val, type_val, **adapt_kwargs)
         elif isinstance(val, str):
             try:
@@ -488,8 +488,8 @@ def adapt_typehints(val, typehint, serialize=False, instantiate_classes=False, p
                 raise ValueError(f'Expected a {typehint} but got "{val}"')
 
     # Registered types
-    elif typehint in registered_types:
-        registered_type = registered_types[typehint]
+    elif get_registered_type(typehint):
+        registered_type = get_registered_type(typehint)
         if serialize:
             if registered_type.is_value_of_type(val):
                 val = registered_type.serializer(val)
diff --git a/jsonargparse/typing.py b/jsonargparse/typing.py
index 2b1279c..31645d0 100644
--- a/jsonargparse/typing.py
+++ b/jsonargparse/typing.py
@@ -3,11 +3,9 @@
 import operator
 import os
 import re
-import uuid
-from datetime import timedelta
 from typing import Any, Callable, Dict, List, Optional, Pattern, Tuple, Type, Union
 from .optionals import final
-from .util import Path
+from .util import get_import_path, import_object, Path
 
 
 __all__ = [
@@ -44,7 +42,7 @@ _operators1 = {
 }
 _operators2 = {v: k for k, v in _operators1.items()}
 
-registered_types: Dict[Tuple, Any] = {}
+registered_types: Dict[Union[str, Type, Tuple], Any] = {}
 
 
 def create_type(
@@ -268,6 +266,7 @@ def register_type(
     deserializer: Optional[Callable] = None,
     deserializer_exceptions: Union[Type[Exception], Tuple[Type[Exception], ...]] = (ValueError, TypeError, AttributeError),
     type_check: Callable = lambda v, t: v.__class__ == t,
+    fail_already_registered: bool = True,
     uniqueness_key: Optional[Tuple] = None,
 ):
     """Registers a new type for use in jsonargparse parsers.
@@ -278,10 +277,12 @@ def register_type(
         deserializer: Function that converts a basic type to an instance of the class. Default None instantiates type_class.
         deserializer_exceptions: Exceptions that deserializer raises when it fails.
         type_check: Function to check if a value is of type_class. Gets as arguments the value and type_class.
+        fail_already_registered: Whether to fail if type has already been registered.
         uniqueness_key: Key to determine uniqueness of type.
     """
     type_wrapper = RegisteredType(type_class, serializer, deserializer, deserializer_exceptions, type_check)
-    if type_class in registered_types:
+    fail_already_registered = globals().get('_fail_already_registered', fail_already_registered)
+    if not uniqueness_key and fail_already_registered and get_registered_type(type_class):
         if type_wrapper == registered_types[type_class]:
             return
         raise ValueError(f'Type "{type_class}" already registered with different serializer and/or deserializer.')
@@ -290,6 +291,26 @@ def register_type(
         registered_types[uniqueness_key] = type_class
 
 
+def register_type_on_first_use(import_path: str, *args, **kwargs):
+    registered_types[import_path] = lambda: register_type(
+        import_object(import_path),
+        *args,
+        **kwargs,
+    )
+
+
+def get_registered_type(type_class):
+    if type_class in registered_types:
+        return registered_types[type_class]
+    try:
+        import_path = get_import_path(type_class)
+        if import_path in registered_types:
+            registered_types.pop(import_path)()
+            return registered_types[type_class]
+    except (AttributeError, ValueError):
+        pass
+
+
 def add_type(type_class: Type, uniqueness_key: Optional[Tuple], type_check: Callable = None):
     assert uniqueness_key not in registered_types
     if type_class.__name__ in globals():
@@ -306,6 +327,8 @@ def is_final_class(cls):
     return getattr(cls, '__final__', False)
 
 
+_fail_already_registered = False
+
 PositiveInt        = restricted_number_type('PositiveInt',        int, ('>', 0),
                                             docstring='int restricted to be >0')
 NonNegativeInt     = restricted_number_type('NonNegativeInt',     int, ('>=', 0),
@@ -332,7 +355,7 @@ Path_drw = path_type('drw', docstring='str pointing to a directory that exists a
 
 register_type(os.PathLike, str, str)
 register_type(complex)
-register_type(uuid.UUID)
+register_type_on_first_use('uuid.UUID')
 
 
 def timedelta_deserializer(value):
@@ -347,7 +370,10 @@ def timedelta_deserializer(value):
     if not match:
         raise_error()
     kwargs = {key: float(val) for key, val in match.groupdict().items()}
+    from datetime import timedelta
     return timedelta(**kwargs)
 
 
-register_type(timedelta, deserializer=timedelta_deserializer)
+register_type_on_first_use('datetime.timedelta', deserializer=timedelta_deserializer)
+
+del _fail_already_registered


Test patch:
diff --git a/jsonargparse_tests/test_core.py b/jsonargparse_tests/test_core.py
index 7a464d5..f22401b 100755
--- a/jsonargparse_tests/test_core.py
+++ b/jsonargparse_tests/test_core.py
@@ -32,7 +32,6 @@ from jsonargparse import (
 from jsonargparse.namespace import meta_keys
 from jsonargparse.optionals import (
     docstring_parser_support,
-    dump_preserve_order_support,
     fsspec_support,
     jsonnet_support,
     jsonschema_support,
@@ -41,7 +40,7 @@ from jsonargparse.optionals import (
 )
 from jsonargparse.typing import NotEmptyStr, Path_fc, Path_fr, PositiveFloat, PositiveInt
 from jsonargparse.util import CaptureParserException, capture_parser, DebugException, null_logger
-from jsonargparse_tests.base import is_posix, responses_activate, responses_available, TempDirTestCase
+from jsonargparse_tests.base import is_cpython, is_posix, responses_activate, responses_available, TempDirTestCase
 
 
 def example_parser():
@@ -680,6 +679,7 @@ class OutputTests(TempDirTestCase):
         self.assertEqual(parser.dump(cfg), 'paths:\n- path1\n- path2\n')
 
 
+    @unittest.skipIf(not is_cpython, 'requires __setattr__ insertion order')
     def test_dump_formats(self):
         parser = ArgumentParser()
         parser.add_argument('--op1', default=123)
@@ -711,8 +711,6 @@ class OutputTests(TempDirTestCase):
         self.assertEqual(parser.dump(parser.parse_args(['--g2.op2=pqr']), skip_default=True), 'g2:\n  op2: pqr\n')
 
 
-    @unittest.skipIf(not dump_preserve_order_support,
-                     'Dump preserve order only supported in python>=3.6 and CPython')
     def test_dump_order(self):
         args = {}
         for num in range(50):
diff --git a/jsonargparse_tests/test_loaders_dumpers.py b/jsonargparse_tests/test_loaders_dumpers.py
index 9a1b817..9a0e1bf 100755
--- a/jsonargparse_tests/test_loaders_dumpers.py
+++ b/jsonargparse_tests/test_loaders_dumpers.py
@@ -6,7 +6,6 @@ from importlib.util import find_spec
 from typing import List
 from jsonargparse import ActionConfigFile, ArgumentParser, set_dumper, set_loader, ParserError
 from jsonargparse.loaders_dumpers import load_value, load_value_context, yaml_dump
-from jsonargparse.optionals import dump_preserve_order_support
 
 
 class LoadersTests(unittest.TestCase):
@@ -43,7 +42,7 @@ class LoadersTests(unittest.TestCase):
         self.assertEqual('{one,two,three}', parser.parse_args(['--val={one,two,three}']).val)
 
 
-    @unittest.skipIf(not dump_preserve_order_support or not find_spec('omegaconf'), 'omegaconf package and CPython required')
+    @unittest.skipIf(not find_spec('omegaconf'), 'omegaconf package is required')
     def test_parser_mode_omegaconf(self):
         parser = ArgumentParser(error_handler=None, parser_mode='omegaconf')
         parser.add_argument('--server.host', type=str)
@@ -66,7 +65,7 @@ class LoadersTests(unittest.TestCase):
         self.assertIn('url: http://localhost:80/', parser.dump(cfg))
 
 
-    @unittest.skipIf(not dump_preserve_order_support or not find_spec('omegaconf'), 'omegaconf package and CPython required')
+    @unittest.skipIf(not find_spec('omegaconf'), 'omegaconf package is required')
     def test_parser_mode_omegaconf_in_subcommands(self):
         subparser = ArgumentParser()
         subparser.add_argument('--config', action=ActionConfigFile)
diff --git a/jsonargparse_tests/test_namespace.py b/jsonargparse_tests/test_namespace.py
index cd50680..7a01194 100755
--- a/jsonargparse_tests/test_namespace.py
+++ b/jsonargparse_tests/test_namespace.py
@@ -100,7 +100,7 @@ class NamespaceTests(unittest.TestCase):
         self.assertNotIn('x..y', ns)
         self.assertNotIn(123, ns)
 
-    @unittest.skipIf(not is_cpython, 'requires dict insertion order')
+    @unittest.skipIf(not is_cpython, 'requires __setattr__ insertion order')
     def test_items_generator(self):
         ns = Namespace()
         ns['a'] = 1
@@ -110,7 +110,7 @@ class NamespaceTests(unittest.TestCase):
         items = list(ns.items())
         self.assertEqual(items, [('a', 1), ('b.c', 2), ('b.d', 3), ('p.q.r', {'x': 4, 'y': 5})])
 
-    @unittest.skipIf(not is_cpython, 'requires dict insertion order')
+    @unittest.skipIf(not is_cpython, 'requires __setattr__ insertion order')
     def test_keys_generator(self):
         ns = Namespace()
         ns['a'] = 1
@@ -120,7 +120,7 @@ class NamespaceTests(unittest.TestCase):
         keys = list(ns.keys())
         self.assertEqual(keys, ['a', 'b.c', 'b.d', 'p.q.r'])
 
-    @unittest.skipIf(not is_cpython, 'requires dict insertion order')
+    @unittest.skipIf(not is_cpython, 'requires __setattr__ insertion order')
     def test_values_generator(self):
         ns = Namespace()
         ns['a'] = 1
diff --git a/jsonargparse_tests/test_typing.py b/jsonargparse_tests/test_typing.py
index 086d2dd..9463004 100755
--- a/jsonargparse_tests/test_typing.py
+++ b/jsonargparse_tests/test_typing.py
@@ -9,6 +9,7 @@ from jsonargparse import ArgumentParser, ParserError
 from jsonargparse.typing import (
     ClosedUnitInterval,
     Email,
+    get_registered_type,
     NonNegativeFloat,
     NonNegativeInt,
     OpenUnitInterval,
@@ -21,7 +22,7 @@ from jsonargparse.typing import (
     restricted_number_type,
     restricted_string_type,
 )
-from jsonargparse.util import object_path_serializer
+from jsonargparse.util import import_object, object_path_serializer
 from jsonargparse_tests.base import mock_module, TempDirTestCase
 
 
@@ -186,6 +187,8 @@ class PathTypeTests(TempDirTestCase):
 class OtherTests(unittest.TestCase):
 
     def test_pickle_module_types(self):
+        for import_path in [x for x in registered_types.keys() if isinstance(x, str)]:
+            get_registered_type(import_object(import_path))
         for otype in registered_types.values():
             if isinstance(otype, RegisteredType) or hasattr(__import__('jsonargparse.typing'), otype.__name__):
                 if isinstance(otype, RegisteredType):
@@ -226,7 +229,7 @@ class OtherTests(unittest.TestCase):
 
 
     def test_timedelta(self):
-        timedelta_type = registered_types[timedelta]
+        timedelta_type = get_registered_type(timedelta)
         for delta_in, delta_out in [
             ('1:2:3', '1:02:03'),
             ('0:05:30', '0:05:30'),