ci: some ci maintenance work #675
Conversation
JoepVanlier
requested review from
alessiamarcolini,
tobiasjj,
rpauszek and
dean0x7d
|
maybe out of scope here but since we are in maintenance mode: now you could also put the flake8 configuration in |
| dependencies = [ | ||
| "pytest>=3.5", | ||
| "h5py>=3.4, <4", | ||
| "numpy>=1.24, <2", # 1.24 is needed for dtype in vstack/hstack (Dec 18th, 2022) |
There was a problem hiding this comment.
Not for this PR, but it reminded me: think about lifting this upper limit soon. numpy v2.1.0 is already out. In general, v2.x brings some really nice performance improvements for Mx Macs and with a much smaller package size (thanks to using the Accelerate SDK).
There was a problem hiding this comment.
Yeah, I will. I'll play around with it soon, but indeed, I don't want to piggyback that now.
pyproject.toml
Outdated
| length_sort_sections = ["stdlib", "thirdparty", "firstparty", "localfolder"] | ||
| lines_between_sections = 1 | ||
|
|
||
| [tool.pytest.ini_options] | ||
| minversion = "3.5" |
There was a problem hiding this comment.
I'm fairly sure that you're using pytest features newer than v3.5. Maybe bump this up to v7 or v8?
There was a problem hiding this comment.
Good catch! You are correct. I quickly bisected and 7.4 is the first one with everything green, so lower bound to that one.
lumicks/pylake/__about__.py
Outdated
| from importlib.metadata import version | ||
|
|
||
| __title__ = "lumicks.pylake" | ||
| __version__ = "1.5.1" | ||
| __version__ = version("lumicks.pylake") |
There was a problem hiding this comment.
I would swap this to work the other way around with pyproject.toml dynamic metadata: https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html#dynamic-metadata
That way, pyproject.toml reads from this file rather than the other way around. IMO this is nicer because users will be doing import lumicks.pylake a lot more often than pip install lumicks.pylake so it's good to avoid extra overhead on the more frequently used path.
You can share __summary__, __url__ and other info similarly.
There was a problem hiding this comment.
Yeah, I inverted the dependency since I didn't manage to make it work with the editable install (it complained about missing dependencies). But I only just realized that I should really just directly pull it from the __about__ file instead. Must have been tired last Friday 😬.
This is actually nicer for a second reason on top of the overhead, since I would indeed want the package code to be leading in the case of an editable install, rather than the time I happened to install it.
I don't think __summary__ and __url__ can be shared through an attribute, and I would prefer not to put them in a separate file, since I feel that adding file reading to __init__ or __about__ is messier than having these properties we virtually never change in two places.
Did you read the small discussion about the filename normalization? For now I upper bounded to |
|
I think it's only Conda. I think you should just go ahead and unlock the upper bound and see. There really isn't any other way in the end. |
| filterwarnings = "error" | ||
|
|
||
| [tool.setuptools.dynamic] | ||
| version = {attr = "lumicks.pylake.__about__.__version__"} |
Why this PR?
The
pyproject.tomlfile is decribed in PEP 621. It is a more modern way of packaging python.In addition, this PR sets up commit hooks. The last commit is just updates from updating
black.