Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added allow_partial parameter #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

percevalw
Copy link
Member

@percevalw percevalw commented Feb 16, 2025

Description

  • Added new Draft type and auto_draft_in_config parameter to register(...) function. This is meant for functions that can only be partially be instantiated by a user, because a required parameter will be provided by the library later.

    For instance:

    • EDS-NLP's ScheduledOptimizer requires the pipeline parameters, which depends on the pipeline being trained
    • EDS-NLP's local trackers (like csv and json) require logging_dir which might be defined in the edsnlp.train function.

    If a callable registered with allow_draft=True is resolved with missing required parameters, a Draft[ReturnType] class is returned. This class can then be instantiated via the draft.instantiate method. We don't instantiate a Draft class via the __call__ method to avoid users mistakenly proceeding with a non instantiated class. Instead, whenever a Draft an attribute or a method is requested on Draft class, outside the Draft few specific attrs/methods, a message error is displayed explaining how this object is not instantiated yet.

    The user can also explicitly instantiate the Draft object via the Class.draft method for classes that have been wrapped with validate_arguments.

TODO later: allow to specify which parameters will be filled eg. Draft[int, ["name"]] declares that the library will fill the "name" parameter.

Checklist

  • If this PR is a bug fix, the bug is documented in the test suite.
  • Changes were documented in the changelog (pending section).

Copy link

github-actions bot commented Feb 16, 2025

Coverage Report

NameStmtsMiss∆ MissCover
TOTAL106718098.31%
Files without new missing coverage
NameStmtsMiss∆ MissCover
confit/utils/settings.py

Was already missing at lines 10-12

         return bool(loads(value))
-     except Exception:
-         return True

102080.00%
confit/registry.py

Was already missing at line 132

         if name is None and hasattr(e.model, "type_"):
-             name = e.model.type_.__module__ + "." + e.model.type_.__qualname__
         e = ConfitValidationError(
Was already missing at lines 640-645
         """
-         entrypoints = importlib_metadata.entry_points()
  ...
-             return entrypoints.get(self.entry_point_namespace, [])
Was already missing at line 670
                 if from_entry_point:
-                     return from_entry_point
             if not catalogue.check_exists(*path):
Was already missing at line 677
                 )
-             return catalogue._get(path)

2347097.01%
confit/config.py

Was already missing at line 126

                     else:
-                         current = current[part]
                 try:
Was already missing at line 155
         if resolve:
-             return config.resolve(registry=registry)
Was already missing at line 376
                     return local_names[parts] + ("." if path.endswith(":") else "")
-                 except KeyError:
                     raise KeyError(path)
Was already missing at line 421
             if not deep and len(loc) > 1:
-                 return obj
Was already missing at line 501
                 return
-             current[path] = val
Was already missing at line 528
                     ):
-                         old[key] = new_val
                     else:
Was already missing at line 564
             elif isinstance(obj, list):
-                 return [rec(v) for v in obj]
             elif isinstance(obj, tuple):
Was already missing at line 566
             elif isinstance(obj, tuple):
-                 return tuple(rec(v) for v in obj)
             elif isinstance(obj, Reference):
Was already missing at line 615
     if isinstance(config_paths, Path):
-         config_paths = [config_paths]

3069097.06%

10 files skipped due to complete coverage.

Coverage success: total of 98.31% is above 98.12% 🎉

@percevalw percevalw force-pushed the allow-partial branch 15 times, most recently from bb7bf91 to aeb6325 Compare February 17, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant