Skip to content

370 improve deprecation path for data paths#373

Merged
popescu-v merged 2 commits intodevfrom
370-improve-deprecation-path-for-data-paths
Apr 2, 2025
Merged

370 improve deprecation path for data paths#373
popescu-v merged 2 commits intodevfrom
370-improve-deprecation-path-for-data-paths

Conversation

@popescu-v
Copy link
Copy Markdown
Collaborator

@popescu-v popescu-v commented Mar 26, 2025

closes #370


TODO Before Asking for a Review

  • Rebase your branch to the latest version of dev (or main for release PRs)
  • Make sure all CI workflows are green
  • When adding a public feature/fix: Update the Unreleased section of CHANGELOG.md (no date)
  • Self-Review: Review "Files Changed" tab and fix any problems you find
  • API Docs (only if there are changes in docstrings, rst files or samples):
    • Check the docs build without warning: see the log of the API Docs workflow
    • Check that your changes render well in HTML: download the API Docs artifact and open index.html
    • If there are any problems it is faster to iterate by building locally the API Docs

@popescu-v popescu-v self-assigned this Mar 26, 2025
@popescu-v popescu-v linked an issue Mar 26, 2025 that may be closed by this pull request
@popescu-v popescu-v force-pushed the 370-improve-deprecation-path-for-data-paths branch from 926d113 to cf2da77 Compare March 26, 2025 15:51
Namely, detect previous-style data paths if:
- they contain '`', and,
- their first fragment (after split on '`') is identical to the name of one
  dictionary from the domain.
@popescu-v popescu-v force-pushed the 370-improve-deprecation-path-for-data-paths branch 4 times, most recently from 36e3d9c to bc5b5ed Compare April 1, 2025 15:04
Copy link
Copy Markdown
Member

@folmos-at-orange folmos-at-orange left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor stuff.

@popescu-v popescu-v force-pushed the 370-improve-deprecation-path-for-data-paths branch from bc5b5ed to afa14d0 Compare April 2, 2025 13:16
More specifically, preprocess the data paths in the
`additional_data_tables` and `output_additional_data_tables` arguments:

- split each path on "`"
- check that each data path fragment after the split is non-empty
- check that the first fragment is identical to the the name of the
  current dictionary (as specified in `dictionary_name` or
  `train_dictionary_name`)

If all these conditions are met, then convert the legacy data path to
the new format:

- drop the current dictionary name fragment from the beginning of the
  path

- join the remaining data path fragments on "/"

Note: This does not handle external tables, but for the impending beta,
this seems like an acceptable tradeoff.

closes #370
@popescu-v popescu-v force-pushed the 370-improve-deprecation-path-for-data-paths branch from afa14d0 to 6142e50 Compare April 2, 2025 13:50
@popescu-v popescu-v merged commit 401c465 into dev Apr 2, 2025
19 checks passed
@popescu-v popescu-v deleted the 370-improve-deprecation-path-for-data-paths branch April 2, 2025 14:10
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.

Improve Deprecation Path for Data-Paths

2 participants