8000 Move to pyproject_toml + package cleanup by davidt0x · Pull Request #549 · brainiak/brainiak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Move to pyproject_toml + package cleanup #549

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

Merged
merged 207 commits into from
Nov 15, 2024
Merged

Move to pyproject_toml + package cleanup #549

merged 207 commits into from
Nov 15, 2024

Conversation

davidt0x
Copy link
Contributor
@davidt0x davidt0x commented Nov 15, 2024

This is a very large PR that began with a simple goal of moving the project over to pyproject TOML standard. Due to continued issues with failing CI jobs because of things like upper pins on old packages, dependencies on deprecated packages (theano), failing mpi tests, among other things; the PR became much more complicated. A rough list of improvements are the following:

  • Moved to pyproject.toml standard
  • Refactor package to generally recommended src layout
  • Moved build to CMake + scikit-build-core
  • Support for more recent pythons (3.12), 3.13 is limited by TF support now
  • Building wheels now on all major platforms (linux, macos ARM/x64, and windows)
  • MPI tests are marked now and run in separate process using a slightly modified fork of https://github.com/minrk/pytest-mpiexec
  • Removed almost all upper pins on packages, only TF probability remains.
  • Conda packages building on linux and mac using mamba build, no windows here because conda-forge TF support on windows seems gone now
  • Removed Theano as a dependency by porting ssrm to use tensforflow instead for differentiation. Tests pass but probably someone should look at this code more carefully to make sure nothing is broken.
  • Fixed a large number of warnings all throughout the code
  • Probably some other small things I am forgetting.

I still think there is considerable work to be done in cleaning up the pr-check.sh, run-tests.sh, and run-checks.sh scripts. Probably better to move these to github actions workflows or something so we can run them on windows. Currently, windows checks are just running pytest.

Seems like this dependency isn't needed.

The hfta example in docs imports it but
never uses it.

It is giving us issues with Python 3.9 and MacOS because it depends on deepdish which depends on tables. Tables dropped 3.9 support.
Hope this fixes issues with finding
library in cibuildwheel repair wheel.
Seems the MPI tests can be flakey.
For now, we just run tests, no pr-check.

I think the next steps should be to get rid
of pr-check.sh, run-checks.sh, and run-tests.sh and replace with actions.
@davidt0x
Copy link
Contributor Author

Hey @mihaic , I think this one is ready, let me know what you think. della_notebooks run needs to be fixed. I will do another PR for it though.

Copy link
Member
@mihaic mihaic left a comment

Choose a reason for hiding this comment

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

Amazing work, @davidt0x!

@mihaic mihaic added this pull request to the merge queue Nov 15, 2024
Merged via the queue into master with commit 8837c18 Nov 15, 2024
43 of 44 checks passed
@mihaic mihaic deleted the pyproject_toml branch November 15, 2024 19:24
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
0