Skip to content

Cherry-pick [2.7] pin fastdigest==0.4.0 due to API changes (#4217)#4291

Open
YuanTingHsieh wants to merge 2 commits intoNVIDIA:mainfrom
YuanTingHsieh:cherry-pick-4217
Open

Cherry-pick [2.7] pin fastdigest==0.4.0 due to API changes (#4217)#4291
YuanTingHsieh wants to merge 2 commits intoNVIDIA:mainfrom
YuanTingHsieh:cherry-pick-4217

Conversation

@YuanTingHsieh
Copy link
Collaborator

Summary

  • pin fastdigest to 0.4.0 in setup.cfg for 2.7 test environments
  • keep 2.7 on the existing TDigest API by reverting temporary compatibility-layer code changes in quantile runtime/tests
  • remove temporary user-facing example API additions so example/job interfaces remain unchanged
  • update docs/READMEs/tutorial notebooks to explicitly state 2.7 quantile workflows should use fastdigest==0.4.0

Test plan

  • python -m py_compile nvflare/app_opt/statistics/df/df_core_statistics.py nvflare/app_opt/statistics/quantile_stats.py tests/unit_test/app_common/statistics/quantile_test.py
  • python -m py_compile examples/advanced/federated-statistics/df_stats/client.py examples/advanced/federated-statistics/df_stats/job.py examples/hello-world/hello-tabular-stats/client.py examples/hello-world/hello-tabular-stats/job.py
  • pytest -q tests/unit_test/app_common/statistics/quantile_test.py (blocked locally: current macOS env segfaults on import numpy)
  • Linux CI / non-mac validation for quantile tests with fastdigest==0.4.0

Notes

  • This PR intentionally avoids user-facing API changes on 2.7.
  • TDigest API migration for newer fastdigest versions will be handled later on main.

Fixes # .

Description

A few sentences describing the changes proposed in this pull request.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Quick tests passed locally by running ./runtest.sh.
  • In-line docstrings updated.
  • Documentation updated.

Copilot AI review requested due to automatic review settings March 11, 2026 01:29
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR pins fastdigest to 0.4.0 in the test extras of setup.cfg and propagates that pinned version requirement across all user-facing documentation (RST docs, READMEs, and Jupyter notebooks), keeping the 2.7 branch stable against the breaking TDigest API changes introduced in newer fastdigest releases.

Key changes:

  • setup.cfg: Adds fastdigest==0.4.0; python_version < "3.14" to the [options.extras_require] test group, but not to test_mac — creating a potential gap if Mac quantile tests are ever unblocked.
  • All documentation touchpoints (federated_statistics_overview.rst, index.rst, two READMEs, two Jupyter notebooks) are updated consistently to direct users to pip install fastdigest==0.4.0.
  • The python_version < "3.14" environment marker in setup.cfg silently excludes the pin on Python 3.14+; this is worth confirming is intentional (e.g., because fastdigest==0.4.0 does not build on that interpreter).

Confidence Score: 4/5

  • Safe to merge with minor follow-up: the test_mac extras omission and the python_version < "3.14" guard should be verified as intentional.
  • All changes are documentation updates and a single dependency pin. No runtime or user-facing API is modified. The only potential issue is that fastdigest==0.4.0 is added to test but not test_mac, which could silently break Mac quantile tests once the unrelated numpy blocker is resolved. The python_version < "3.14" guard is also worth confirming as intentional.
  • setup.cfg — verify that the test_mac omission and python_version < "3.14" guard are intentional.

Important Files Changed

Filename Overview
setup.cfg Pins fastdigest==0.4.0 (with a python_version < "3.14" guard) to the test extras only; test_mac is not updated, creating a potential inconsistency.
docs/examples/federated_statistics_overview.rst Adds a pip install fastdigest==0.4.0 code block and a note that the newer TDigest API will be adopted in a later release; straightforward documentation update.
docs/hello-world/hello-tabular-stats/index.rst Updates the fastdigest install instruction to specify the exact pinned version fastdigest==0.4.0.
examples/advanced/federated-statistics/df_stats/README.md Adds a pip install fastdigest==0.4.0 bash code block under the quantile description section; clean, correct change.
examples/hello-world/hello-tabular-stats/README.md Updates install instruction from a generic fastdigest mention to the pinned fastdigest==0.4.0.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["pip install nvflare test"] --> B{"python_version < 3.14?"}
    B -- Yes --> C["fastdigest==0.4.0 installed"]
    B -- No --> D["fastdigest NOT installed"]
    C --> E["Quantile tests pass on Linux CI"]
    D --> F["Quantile tests fail on Python 3.14+"]

    G["pip install nvflare test_mac"] --> H["fastdigest NOT in test_mac extras"]
    H --> I["Quantile tests may fail on Mac CI"]
Loading

Last reviewed commit: 01bf045


pip install fastdigest==0.4.0

Newer ``fastdigest`` releases changed the TDigest API and are planned to be adopted in a later branch.
Copy link
Contributor

Choose a reason for hiding this comment

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

User-facing phrasing references internal branch concept

The phrase "planned to be adopted in a later branch" uses Git branching terminology that is internal to the project and may confuse end users. Consider replacing it with version-oriented language such as "a future release" to keep the documentation audience-appropriate.

Suggested change
Newer ``fastdigest`` releases changed the TDigest API and are planned to be adopted in a later branch.
Newer ``fastdigest`` releases changed the TDigest API and are planned to be adopted in a future release.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Pins fastdigest to a specific version for the 2.7 branch to avoid TDigest API drift, and updates user-facing docs/tutorials to consistently instruct fastdigest==0.4.0 for quantile workflows.

Changes:

  • Add fastdigest==0.4.0 to the test extras in setup.cfg.
  • Update multiple READMEs/docs/notebooks to explicitly pin fastdigest==0.4.0 for quantile examples.
  • Add a pinned-install snippet to the advanced federated-statistics README.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
setup.cfg Pins fastdigest==0.4.0 under test extras to keep test envs on the older TDigest API.
examples/tutorials/.../federated_statistics_with_tabular_data.ipynb Updates tutorial text to instruct installing fastdigest==0.4.0 for quantiles.
examples/hello-world/hello-tabular-stats/df_stats.ipynb Updates notebook instructions to pin fastdigest==0.4.0.
examples/hello-world/hello-tabular-stats/README.md Updates README to pin fastdigest==0.4.0.
examples/advanced/federated-statistics/df_stats/README.md Adds explicit “install and pin” snippet for fastdigest==0.4.0.
docs/hello-world/hello-tabular-stats/index.rst Updates docs to pin fastdigest==0.4.0.
docs/examples/federated_statistics_overview.rst Adds guidance to pin fastdigest==0.4.0 and notes API changes in newer releases.

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

"> **Installing fastdigest**\n",
">\n",
"> If you intend to calculate quantiles, you need to install fastdigest. the fastdigest not included in the requirements.txt file. If you are not calculating quantiles, you can skip this step.\n",
"> If you intend to calculate quantiles, install `fastdigest==0.4.0`. the fastdigest not included in the requirements.txt file. If you are not calculating quantiles, you can skip this step.\n",
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The updated sidebar sentence has grammatical issues (e.g., "the fastdigest not included") and double spacing after the period. Please revise to a complete, capitalized sentence (e.g., "fastdigest is not included in requirements.txt").

Suggested change
"> If you intend to calculate quantiles, install `fastdigest==0.4.0`. the fastdigest not included in the requirements.txt file. If you are not calculating quantiles, you can skip this step.\n",
"> If you intend to calculate quantiles, install `fastdigest==0.4.0`. The `fastdigest` package is not included in the `requirements.txt` file. If you are not calculating quantiles, you can skip this step.\n",

Copilot uses AI. Check for mistakes.
## Summary
- pin `fastdigest` to `0.4.0` in `setup.cfg` for `2.7` test environments
- keep `2.7` on the existing TDigest API by reverting temporary
compatibility-layer code changes in quantile runtime/tests
- remove temporary user-facing example API additions so example/job
interfaces remain unchanged
- update docs/READMEs/tutorial notebooks to explicitly state `2.7`
quantile workflows should use `fastdigest==0.4.0`

## Test plan
- [x] `python -m py_compile
nvflare/app_opt/statistics/df/df_core_statistics.py
nvflare/app_opt/statistics/quantile_stats.py
tests/unit_test/app_common/statistics/quantile_test.py`
- [x] `python -m py_compile
examples/advanced/federated-statistics/df_stats/client.py
examples/advanced/federated-statistics/df_stats/job.py
examples/hello-world/hello-tabular-stats/client.py
examples/hello-world/hello-tabular-stats/job.py`
- [ ] `pytest -q tests/unit_test/app_common/statistics/quantile_test.py`
(blocked locally: current macOS env segfaults on `import numpy`)
- [ ] Linux CI / non-mac validation for quantile tests with
`fastdigest==0.4.0`

## Notes
- This PR intentionally avoids user-facing API changes on `2.7`.
- TDigest API migration for newer `fastdigest` versions will be handled
later on `main`.
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.

3 participants