Skip to content

[ENH] Add exclude_tags parameter to all_objects #521#526

Open
direkkakkar319-ops wants to merge 3 commits intosktime:mainfrom
direkkakkar319-ops:issue#521-DirekKakkar
Open

[ENH] Add exclude_tags parameter to all_objects #521#526
direkkakkar319-ops wants to merge 3 commits intosktime:mainfrom
direkkakkar319-ops:issue#521-DirekKakkar

Conversation

@direkkakkar319-ops
Copy link

Reference Issues/PRs

Fixes #521

What does this implement/fix? Explain your changes.

all_objects had a filter_tags parameter to include objects by tag
but no symmetric exclude_tags to filter them out. This PR adds that.

3 localized changes in skbase/lookup/_lookup.py:

  • Line 755 (signature) — added exclude_tags=None parameter
  • Lines 822-834 (docstring) — added parameter docs and usage examples
  • Lines 947-953 (logic) — added exclusion filter reusing the existing
    _filter_by_tags function with inverted matching — clean and minimal

5 new tests added in skbase/tests/test_lookup.py:

  • test_exclude_tags_none_is_noopexclude_tags=None returns all objects
  • test_exclude_tags_empty_dict_is_noopexclude_tags={} returns all objects
  • test_exclude_tags_excludes_matching — correctly excludes by tag value
  • test_exclude_tags_combined_with_filter_tagsfilter_tags + exclude_tags
    work together correctly
  • test_exclude_tags_multiple_conditions — multiple tag conditions work as conjunction

No changes needed to skbase/lookup/__init__.py (already re-exports
all_objects directly) or get_package_metadata (different interface).

Does your contribution introduce a new dependency? If yes, which one?

No new dependencies introduced.

What should a reviewer concentrate their feedback on?

  • The inverted _filter_by_tags logic at lines 947-953 — is this the
    cleanest way to implement exclusion or would a separate helper be preferred?
  • The 5 new tests — are there any edge cases missing that should be covered?
  • Docstring examples — are they clear and consistent with existing examples
    in the file?

Any other comments?

  • black — passes
  • flake8 — 0 errors
  • pytest — all 6 tests pass (1 existing + 5 new)

The implementation is intentionally minimal and localized to _lookup.py
only, reusing the existing _filter_by_tags helper rather than introducing
any new logic.

PR checklist

For all contributions
  • I've reviewed the project documentation on contributing
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [CI/CD], [MNT], [DOC], or [BUG] indicating whether
    the PR topic is related to enhancement, CI/CD, maintenance, documentation, or a bug.
For code contributions
  • Unit tests have been added covering code functionality
  • Appropriate docstrings have been added (see documentation standards)
  • New public functionality has been added to the API Reference

@direkkakkar319-ops direkkakkar319-ops marked this pull request as ready for review March 11, 2026 03:35
@direkkakkar319-ops
Copy link
Author

Hey, @fkiraly @felipeangelimvieira I'm a new contributor in this org and just raised this PR for Issue #521
Would love it if someone could review it when they get a chance. Thanks.

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.

[ENH] Add exclude_tags parameter to all_objects

1 participant