Pyproject code refactor #289
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## develop #289 +/- ##
==========================================
Coverage ? 91.48%
==========================================
Files ? 44
Lines ? 2372
Branches ? 0
==========================================
Hits ? 2170
Misses ? 202
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Great work !
We might want to push it further by using a src layout to tidy things up further and avoiding distributing test, examples and other files with the python package (see https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/ for more info)
Also, the signals for the CI needs to be update I guess in the github settings (to avoid the Expected — Waiting for status to be reported )
MANIFEST.in
Outdated
There was a problem hiding this comment.
I am not so sure we need a MANIFEST.in (which is for source distribution) anymore. Everything should be handled by pyproject.toml.
There was a problem hiding this comment.
I was trying to find some concrete information on this, if you have any references for this point I will check them out.
There was a problem hiding this comment.
Looking at this issue again for another project of mine I found the following references:
TLDR: As we are in a python-only codebase, with no extra data files, there is no need for the MANIFEST.in everything is handled by setuptools.
| ) | ||
|
|
||
| __version__ = _version | ||
| __version__ = "1.6.1" |
There was a problem hiding this comment.
Instead of hardcoding the version, use setuptools-scm plugin and the following
| __version__ = "1.6.1" | |
| try: | |
| # -- Distribution mode -- | |
| # import from _version.py generated by setuptools_scm during release | |
| from ._version import version as __version__ | |
| except ImportError: | |
| # -- Source mode -- | |
| # use setuptools_scm to get the current version from src using git | |
| from setuptools_scm import get_version as _gv | |
| from os import path as _path | |
| __version__ = _gv(_path.join(_path.dirname(__file__), _path.pardir)) |
There was a problem hiding this comment.
Yeah, this was always just temporary, but nice implementation! I will quick look at what the current consensus is on this point before committing, but I like this.
| authors = [ | ||
| { "name" = "Samuel Farrens", "email" = "[email protected]"}, | ||
| ] | ||
| maintainers = [ | ||
| { "name" = "Samuel Farrens", "email" = "[email protected]" }, | ||
| ] |
There was a problem hiding this comment.
Should'nt be more people in the list 😇 ?
There was a problem hiding this comment.
Yes, of course! I am still playing around with the formatting and metadata. I was think for the authors section we could put something like "The CEA-COSMIC Team" or "The ModOpt Team" to be a more inclusive term for everyone who has contributed. Any thoughts/suggestions are certainly welcome.
I will explicitly add you as a maintainer before I finalise this PR.
| [tool.setuptools.dynamic] | ||
| dependencies = {file = ["requirements.txt"]} | ||
| version = {attr = "modopt.__version__"} |
There was a problem hiding this comment.
I would rather declare the dependencies directly in the pyproject.toml file.
And use setuptools-scm to track the version (see my other comment on modopt/__init__.py
| [tool.setuptools.dynamic] | |
| dependencies = {file = ["requirements.txt"]} | |
| version = {attr = "modopt.__version__"} | |
| [tool.setuptools_scm] | |
| write_to = "modopt/_version.py" | |
| version_scheme = "python-simplified-semver" | |
| local_scheme="no-local-version" |
There was a problem hiding this comment.
OK, I will look into this.
Co-authored-by: Pierre-Antoine Comby <[email protected]>
Co-authored-by: Pierre-Antoine Comby <[email protected]>
|
Thanks for the feedback @paquiteau, I am still playing around with this PR, but I like these suggestions. I will play around some more this week and ping you when the PR is ready for final review. |
Summary
This PR refactors the codebase to simplify maintenance and development.
setup.pyandsetup.cfgand addedpyproject.toml.pylintrc,.pyup.yml,develop.txt,docs/requirements.txt,notebooksmodopt(i.e. only style changes, no changes to code or tests)To Do
README.mdand installation instructions