-
Notifications
You must be signed in to change notification settings - Fork 16
Migrate from poetry to uv, sphinx to mkdocs #474
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #474 +/- ##
==========================================
- Coverage 79.65% 74.95% -4.70%
==========================================
Files 166 166
Lines 10554 10554
==========================================
- Hits 8407 7911 -496
- Misses 2147 2643 +496 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
It looks like there are still a number of places in the repository that reference the old poetry instructions. I'd suggest also updating them there to make sure we're consistent in guidance throughout, even in the research repositories folders etc. |
|
It looks like we also need to update the link |
|
In CONTRIBUTING.md the note/alert renderings are broken for the note: and |
|
It looks like the latex/math renderings are also broken. See |
|
The code rendering for the docs is also broken. See the init function for the |
emersodb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall a really needed migration from Poetry to UV. This will definitely help us a lot. A few minor comments and some other things that have broken with the move from Sphinx to mkdocs. I fixed some of them, but there are others that require a bit of looking into.
| dm-tree = "^0.1.9" | ||
| cmake = "^3.31.6" | ||
| matplotlib = "^3.10.1" | ||
| keras = "^3.11.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems as though keras was removed here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if keras is actually used in the library? i don't see it being imported anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. It may not be. If the tests pass then we're probably good to remove it from the core. Might want to update the comment though that says it's being included in dev ("so keras is also in dev group")
The migration from poetry to uv resulted in different resolved versions for several packages (torchtext 0.14.1→0.18.0, torchdata 0.7.1→0.11.0, and others), which changed the training dynamics and thus the expected metrics values. Updated expected metrics to match the new deterministic results produced with uv-resolved package versions. The test now passes consistently with seed=42 using the improved package versions. Changes: - Updated all server metrics (3 rounds) with tolerance of 0.002 - Updated all client metrics (3 rounds) with tolerance of 0.002 - Values are from multiple test runs confirming consistency
…mpler The test was failing with different metrics on each run because the DirichletLabelBasedSampler was using an unseeded numpy random state. When hash_key=None (the default), the sampler calls np.random.dirichlet() which uses the global numpy RNG state, bypassing the seed set by set_all_random_seeds(). Changes: - Pass seed to MnistApflClient and store it as an instance variable - Pass hash_key=self.seed when creating DirichletLabelBasedSampler - Update expected metrics to new deterministic values with seed=42 - Reduce tolerance from 0.002 to 0.001 now that results are deterministic This fixes the root cause of non-determinism between runs and between macOS/Linux platforms.
This pull request makes a comprehensive migration from Poetry to uv for dependency management, updates the development and documentation workflows to use uv and MkDocs, and modernizes the documentation structure and build process. It removes legacy Sphinx and Poetry configurations, updates CI/CD pipelines, and provides clearer developer instructions.
The most important changes are:
Dependency Management and Development Workflow
CONTRIBUTING.MD,docs/contributing.md,.pre-commit-config.yaml,.python-version) [1] [2] [3] [4]Continuous Integration and Deployment
tests.yaml,smoke_tests.yaml,static_code_checks.yaml,publish_and_release.yml) to use uv for dependency installation, environment setup, and running scripts/tests, removing all Poetry-related steps and caches. [1] [2] [3] [4] [5] [6] [7].github/workflows/docs.ymlfor building and deploying docs with uv and MkDocs, and removed legacy Sphinx-based workflows (docs_build.yml,docs_publish.yml). [1] [2] [3]Documentation System Modernization
docs/Makefile), updating the documentation structure, and adding new guides for building and serving docs locally with uv and MkDocs. (docs/README.md,docs/api.md,docs/examples/index.md) [1] [2] [3] [4]mkdocs.ymland clarified the directory structure and build instructions.General Cleanup
These changes collectively modernize the project's developer experience, streamline CI/CD, and make documentation easier to build and maintain.