Add validation for include filters in CodeList by dc-almeida · Pull Request #573 · IAMconsortium/nomenclature

dc-almeida · 2026-03-17T16:03:45Z

Closes #551. Introduce validation for include filters used in nomenclature.yaml for codelists and mappings to ensure that all specified filters correspond to existing codes.
For codelists, include filters by code attributes, for mappings it filters by model name.

… `filter`

dc-almeida · 2026-03-24T11:40:30Z

Added validation for mappings, as a necessary step when reading nomenclature.yaml (if mappings files include regions not present in external repos, raises).
Logs warning if no model mappings found in external repository, but doesn't raise (e.g.: legacy-definitions).
Refactored RegionAggregationMapping instantiation from file when creation a RegionProcessor to output an exception group instead of raising on first exception.
Updated ruff because it was crashing on my VSCode and realised it was a quite old version.
Added a test suggested by Copilot mocking fetch_repos which made me think might be a good method to apply in other tests to avoid the overhead of importing entire repos.
Added pydantic validation that closes #517

…ithin 'repository'

phackstock · 2026-03-24T15:01:37Z

As an aside to this PR, my two cents on mocking external resources:

Added a test suggested by Copilot mocking fetch_repos which made me think might be a good method to apply in other tests to avoid the overhead of importing entire repos.

I'm always torn when it comes to mocking external resources/services/etc... On the one hand I agree that in order to speed up the tests, it's a good idea to mock things. On the other hand sometimes what you want is precisely an integration type test where you actually test the interaction of your piece of code with external resources.
So in short, in my opinion, mocking can be a good idea but not always. Sometimes you can unknowingly skip bugs by not actually interacting with external services and resources.

Copilot

Pull request overview

Adds validation to catch typos/mismatches in nomenclature.yaml include/exclude filters by ensuring include filters/patterns actually match existing codes/mappings, with accompanying regression tests.

Changes:

Add validation that repository include filters in CodeList match at least one code, raising an ExceptionGroup otherwise.
Add validation that mapping include patterns match at least one model in the mappings repository.
Add tests and new fixture config files to cover wrong nesting level, missing codes, missing hierarchies, and missing mappings.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`nomenclature/codelist.py`	Validates include filters against loaded codes and updates filtering types/signature.
`nomenclature/config.py`	Adds config-level validation for mapping include patterns after fetching repos.
`nomenclature/processor/region.py`	Collects mapping parse errors from external repos instead of failing fast.
`tests/test_codelist.py`	Adds tests asserting missing-code/missing-hierarchy includes raise grouped errors.
`tests/test_config.py`	Adds tests for include/exclude at wrong YAML nesting level and missing mapping includes.
`tests/data/config/*.yaml`	New config fixtures for the new validation test cases.
`pyproject.toml` / `poetry.lock`	Updates `ruff` dev dependency and lockfile.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dc-almeida · 2026-03-24T15:15:09Z

As an aside to this PR, my two cents on mocking external resources:

Added a test suggested by Copilot mocking fetch_repos which made me think might be a good method to apply in other tests to avoid the overhead of importing entire repos.

I'm always torn when it comes to mocking external resources/services/etc... On the one hand I agree that in order to speed up the tests, it's a good idea to mock things. On the other hand sometimes what you want is precisely an integration type test where you actually test the interaction of your piece of code with external resources. So in short, in my opinion, mocking can be a good idea but not always. Sometimes you can unknowingly skip bugs by not actually interacting with external services and resources.

Agreed, the suggestion here would be to apply this for those tests that precisely are only importing external repos to access their data but where what matters is the data and not the connection.

…g` and add test

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

danielhuppmann

I used this feature this morning when implementing iiasa/prisma-workflow#18, very useful! But the error messages could be more precise, see some suggestion inline. And it's not clear to me how models are validated.

danielhuppmann · 2026-03-26T07:28:55Z

+        for exp in expected:
+            assert excinfo.group_contains(
+                ValueError,
+                match=r"No codes found for include filter: " + exp,


Suggested change

match=r"No codes found for include filter: " + exp,

match=r"No regions found for include-filter: " + exp,

danielhuppmann · 2026-03-26T07:29:52Z

+    )
+    try:
+        with pytest.RaisesGroup(
+            ValueError, ValueError, match="Include filter validation failed"


Suggested change

ValueError, ValueError, match="Include filter validation failed"

ValueError, ValueError, match="Importing regions from external repository failed"

I think it will have to be a more generic "Importing definitions..." because other dimensions can also be used in the include filter and the method is applicable to any type of codelist. It still distinguishes from definitions vs. mappings and the name of a variable vs. a region will be self-evident.

danielhuppmann · 2026-03-26T07:30:16Z

+
+    try:
+        with pytest.RaisesGroup(
+            ValueError, match="Include filter validation failed"


Suggested change

ValueError, match="Include filter validation failed"

ValueError, match="Importing mappings from external repository failed"

danielhuppmann · 2026-03-26T07:30:51Z

+        for exp in expected:
+            assert excinfo.group_contains(
+                ValueError,
+                match=r"No codes found for include filter: " + exp,


Suggested change

match=r"No codes found for include filter: " + exp,

match=r"No mapping found for include-filter: " + exp,

danielhuppmann · 2026-03-26T07:34:43Z

            repo.fetch_repo(target_folder / repo_name)

+    def validate_mapping_includes(self) -> None:
+        """Validate that all mapping include patterns match at least one model."""


This is not clear to me, where is the list of models coming from?

The all_models variable, the list of models is compiled from reading all external repo mappings files. So it checks all the models listed on all imported mappings files to see if there are filters that don't apply to any, and if so, raises.

It's not the most effective way (when a RegionProcessor is instantiated, the mapping files are read again), but without some big refactoring it was the only way I saw of raising the error as soon as possible (when nomenclature.yaml is read) and where all the necessary info was gathered (the include filter and the repo mappings to match against).

Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>

dc-almeida added 2 commits March 17, 2026 16:06

Add validation for include filters in CodeList and corresponding tests

7f78773

Refactor include filter validation into separate method to not affect…

089329a

… `filter`

dc-almeida added the enhancement New feature or request label Mar 17, 2026

dc-almeida self-assigned this Mar 17, 2026

dc-almeida added 3 commits March 24, 2026 10:11

Update ruff

8f8bf76

Add model mapping include filter validation and test

373ec26

Refactor test to use mock and avoid shutil operations

368c93a

dc-almeida marked this pull request as ready for review March 24, 2026 11:40

dc-almeida and others added 3 commits March 24, 2026 14:49

Add validation to ensure 'include' and 'exclude' filters are nested w…

01b3ae2

…ithin 'repository'

Merge branch 'main' into feature/include-validation

708866d

Poetry lock

54f557c

phackstock requested review from Copilot and phackstock March 24, 2026 14:56

Copilot started reviewing on behalf of phackstock March 24, 2026 14:56 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Comment thread nomenclature/config.py Outdated

Comment thread tests/test_codelist.py Outdated

Comment thread tests/test_codelist.py Outdated

Comment thread nomenclature/codelist.py Outdated

Comment thread nomenclature/codelist.py

Comment thread nomenclature/codelist.py

dc-almeida and others added 5 commits March 24, 2026 16:34

Change exclude argument in filter_codes to use default factory pattern

832b01a

Ensure backwards compatibility for filter_codes arguments

b68d4c4

Enhance error handling for mapping file parsing in `NomenclatureConfi…

378d564

…g` and add test

Apply suggestions from code review

3f1b6e4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Make ruff

68be283

dc-almeida requested a review from Copilot March 24, 2026 16:32

Copilot started reviewing on behalf of dc-almeida March 24, 2026 16:32 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Comment thread nomenclature/codelist.py Outdated

Comment thread nomenclature/config.py Outdated

Comment thread nomenclature/config.py

Comment thread tests/test_config.py Outdated

dc-almeida and others added 4 commits March 25, 2026 12:23

Apply suggestion

6f90f1c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Remove blank line

8b75460

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Change ExceptionGroup to NoTracebackExceptionGroup

ab7b762

Make ruff

dbfa219

danielhuppmann reviewed Mar 26, 2026

View reviewed changes

dc-almeida and others added 3 commits March 26, 2026 10:01

Improve error messages for definitions include-filter validation

f5850f1

Add newline

66bd090

Co-authored-by: Daniel Huppmann <dh@dergelbesalon.at>

Update error messages in tests

69183e1

dc-almeida requested a review from danielhuppmann March 26, 2026 09:17

	match=r"No codes found for include filter: " + exp,
	match=r"No regions found for include-filter: " + exp,

	ValueError, ValueError, match="Include filter validation failed"
	ValueError, ValueError, match="Importing regions from external repository failed"

	ValueError, match="Include filter validation failed"
	ValueError, match="Importing mappings from external repository failed"

	match=r"No codes found for include filter: " + exp,
	match=r"No mapping found for include-filter: " + exp,

Conversation

dc-almeida commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dc-almeida commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phackstock commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dc-almeida commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielhuppmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

danielhuppmann Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

danielhuppmann Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

dc-almeida Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

danielhuppmann Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

danielhuppmann Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

danielhuppmann Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

dc-almeida Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dc-almeida commented Mar 17, 2026 •

edited

Loading

dc-almeida commented Mar 24, 2026 •

edited

Loading