Skip to content

Conversation

@bwpriest
Copy link
Contributor

Addresses issue #516 by replacing setup.py with a pyproject.toml. Also updated some Actions and docs accordingly. Will hopefully play nice with ReadTheDocs but there could still be issues.

@bwpriest
Copy link
Contributor Author

bwpriest commented May 14, 2025

I understand why the admm wrapper tests are failing - mpisppy/tests/test_stoch_admmWrapper.py is trying to import ${REPO_ROOT}/examples/distr/dist_data.py, but examples/ is not installed as a package.

I believe this can be fixed by moving /examples into mpisppy/examples, adding __init__.py in appropriate places, and updating the imports, but this is a much larger change that I intended to make. It would also have possibly undesirable outcomes, e.g., /examples would then be distributed as mpisppy.examples in wheels deployed to pip.

I'm actually not sure why these imports worked in the original setup.py version, as setuptools.find_packages() does not find the /examples or anything therein. So, I am at an impasse as to what to do here. Everything else appears unaffected. Any thoughts, @bknueven?

@bknueven
Copy link
Collaborator

I understand why the admm wrapper tests are failing - mpisppy/tests/test_stoch_admmWrapper.py is trying to import ${REPO_ROOT}/examples/distr/dist_data.py, but examples/ is not installed as a package.

I believe this can be fixed by moving /examples into mpisppy/examples, adding __init__.py in appropriate places, and updating the imports, but this is a much larger change that I intended to make. It would also have possibly undesirable outcomes, e.g., /examples would then be distributed as mpisppy.examples in wheels deployed to pip.

I'm actually not sure why these imports worked in the original setup.py version, as setuptools.find_packages() does not find the /examples or anything therein. So, I am at an impasse as to what to do here. Everything else appears unaffected. Any thoughts, @bknueven?

Thanks for your efforts on this @bwpriest. My preference for fixing this test would be to pull / copy the example it needs into the mpisppy/tests/examples directory. We already do this, at the risk of be duplicative, but it avoids installing every example as part of the package.

@bwpriest
Copy link
Contributor Author

My preference for fixing this test would be to pull / copy the example it needs into the mpisppy/tests/examples directory. We already do this, at the risk of be duplicative, but it avoids installing every example as part of the package.

Thanks @bknueven for the suggestion! I'm glad to hear that there is already a protocol in place. I'll take a stab at this shortly.

@bwpriest
Copy link
Contributor Author

Thanks @bknueven for your help! I believe this PR is ready. I've verified that the warning discussed in issue #516 is now mitigated.

Copy link
Collaborator

@DLWoodruff DLWoodruff left a comment

Choose a reason for hiding this comment

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

Thanks! These are valuable changes.

@DLWoodruff DLWoodruff merged commit 8262292 into Pyomo:main May 14, 2025
19 checks passed
@bwpriest bwpriest deleted the feature/pyproject_toml branch May 14, 2025 20: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.

3 participants