Skip to content
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

Remove versions from README.rst #12889

Closed
wants to merge 3 commits into from
Closed

Conversation

cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Oct 9, 2024

I've removed the dependency list, because it is constantly out of sync with the actual requirements. In addition, I don't think that we need to feature them that prominently in the README, since all the information is contained in the metadata anyway.

@larsoner
Copy link
Member

larsoner commented Oct 9, 2024

Historically I think we wanted them in the README since you see it clearly when you land on the MNE-Python repo. It does look like pandas at least lists dependencies in their README, so we're not alone in this. It would be nice to at least list/keep the minimum required dependencies. Those don't change all that often. The optional ones do and will much more easily get out of date -- and we have typically nice error messages when you don't have them -- so I'm okay with removing those.

One problem with the current PR is that currently the README is actually the source of truth for our minimums in practice, believe it or not! See for example https://github.com/mne-tools/mne-python/wiki/How-to-make-a-major-or-minor-release#bump-minimums and release PRs like https://github.com/mne-tools/mne-python/pull/12798/files where those versions change concurrently with our code. So we'd need a new process for this. I guess it could be to keep everything in pyproject.toml?

@cbrnr
Copy link
Contributor Author

cbrnr commented Oct 9, 2024

Ah yes, we should move this to pyproject.toml then (there are some version specifiers in there already, but I didn't know that we actually used our README).

I'm fine with listing the core dependencies, these will have to be manually synced on every release (or whenever someone modifies them in pyproject.toml).

@drammock
Copy link
Member

drammock commented Oct 9, 2024

We already parse package metadata (ultimately from pyproject.toml) for syncing min python version in our docs:

mne-python/doc/conf.py

Lines 1297 to 1300 in 53f258d

# -- Dependency info ----------------------------------------------------------
min_py = metadata("mne")["Requires-Python"].lstrip(" =<>")
rst_prolog += f"\n.. |min_python_version| replace:: {min_py}\n"

We could do something similar with our README (i.e., a pre-commit hook that updates the README whenever min versions are changed in pyproject.toml)

@cbrnr
Copy link
Contributor Author

cbrnr commented Oct 9, 2024

We already parse package metadata (ultimately from pyproject.toml) for syncing min python version in our docs:
[...]
We could do something similar with our README (i.e., a pre-commit hook that updates the README whenever min versions are changed in pyproject.toml)

I wouldn't do that, let's not add more magic to things that could be simple. If someone needs to know the exact requirements, they should refer to pyproject.toml, the canonical and standardized location for such information.

@drammock
Copy link
Member

drammock commented Oct 9, 2024

I wouldn't do that, let's not add more magic to things that could be simple. If someone needs to know the exact requirements, they should refer to pyproject.toml, the canonical and standardized location for such information.

I guess we have different definitions of "magic" 🧙🏻 --- to me, this is "sensible automation to reduce the manual work needed to keep things from getting out of sync / inaccurate". I also think there are many users who might end up on our repo webpage and not know what pyproject.toml is (much less to know that it is where they should look for dependency info --- especially given that there's a file called "READ ME" that contains similar information!)

So I am +1 for automating the pyproject.toml -> README.md synchronization (for key dependencies only). If folks aren't on board with that, then I'd rather remove the info from README entirely than have to manually keep it in sync.

@cbrnr
Copy link
Contributor Author

cbrnr commented Oct 9, 2024

+1 for removing entirely from README.

@larsoner
Copy link
Member

larsoner commented Oct 9, 2024

To me pyproject.toml is a dev thing and README and our website are user-facing so it would be good to have the min deps listed in the README still. I don't think the magic to update would be all that complicated, and so far over the last few months the stuff we've added with autofix and via GHA have worked pretty well and reduced maintenance much more than they have added to it (net benefit), so I'm happy to have a bit more magic!

@cbrnr cbrnr closed this Oct 10, 2024
@cbrnr cbrnr deleted the update-readme branch October 10, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants