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

Improve integration mapping chain resolution #526

Merged

Conversation

danielpgross
Copy link
Collaborator

@danielpgross danielpgross commented Jan 10, 2025

tl;dr

Improves integration integration mapping chain resolution:

  • Fixes behaviour for non-consecutive chained mappings
  • Improves error messaging

Changes

#517 introduced automatic resolution of integration mappings to the current taxonomy, taking into account all newer mappings.

While working on #525, we discovered a flaw in that behaviour: subsequent mappings were only taken into account if they appeared in the next immediate version, but not versions after that. For example, 2022-02 would take into account mappings from 2024-07, but not 2024-10.

We also discovered that in case a category fails to be resolved through this process, the error message is cryptic and not actionable.

This PR:

  • Changes the resolution algorithm to check all subsequent versions, not just the immediate next one
  • Adds a check for resolution failure, raising a clear error
  • Greatly improves unit tests to ensure non-consecutive mapping cases are covered

Copy link
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@danielpgross danielpgross marked this pull request as ready for review January 10, 2025 16:30
Copy link
Collaborator Author

danielpgross commented Jan 10, 2025

Merge activity

  • Jan 10, 11:37 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Jan 10, 11:41 AM EST: A user merged this pull request with Graphite.

@danielpgross danielpgross merged commit 3ace3f2 into main Jan 10, 2025
5 checks passed
@danielpgross danielpgross deleted the 01-10-improve_integration_mapping_chain_resolution branch January 10, 2025 16:41
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.

2 participants