Skip to content

Conversation

@larsoner
Copy link
Member

@larsoner larsoner commented Aug 9, 2024

  1. Format top of lines everywhere in MNE except examples/tutorials as:
    # Authors: The MNE-Python contributors.
    # License: BSD-3-Clause
    # Copyright the MNE-Python contributors.
    
  2. In tutorials/examples, existing Authors: lines will be preserved, otherwise lines will be as above.
  3. Add ensure_headers.py to autofix so the next time we add a file it will get this info automatically. I expect it to almost immediately push a (large!) commit to this PR. So after we merge this, we can add the squashed commit to .git-blame-ignore-revs. This will lose the history on ensure_headers.py but I don't think that really matters -- I think I'm the only author at the moment and it's just a few lines changed anyway.

NB sklearn went with:

# Authors: The scikit-learn developers
# SPDX-License-Identifier: BSD-3-Clause

I chose "contributors" instead of "developers" to follow our existing convention. I also chose not to modify the License: line. Happy to change those here or elsewhere if there is a standard we should follow @drammock !

Closes #11605

@larsoner larsoner added this to the 1.8 milestone Aug 9, 2024
@larsoner larsoner requested a review from drammock as a code owner August 9, 2024 18:38
@larsoner larsoner force-pushed the headers branch 2 times, most recently from 8783b44 to 20c6da5 Compare August 9, 2024 18:46
@larsoner
Copy link
Member Author

larsoner commented Aug 9, 2024

Note that this did change some examples and tutorials, namely adding the boilerplate "Authors" line to the .py files that had no authorship, which seems like a good idea.

Copy link
Member

Choose a reason for hiding this comment

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

for this file (similar for conf.py, and probably others) I feel like one of two things should happen:

  1. the authors/license/copyright should come before the other line-comments, and be separated by a blank line.
  2. the other line-comments should be converted to a triple-quoted string, like what we see in tools/dev/ensure_headers.py

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay this revealed a lot more issues / inconsistencies actually, should be much better now!

Copy link
Member

Choose a reason for hiding this comment

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

unrelated comment: why is this a .py file? It looks like plain text, one entity per line. Also it's first-line comment "testing stuff" is not very helpful

Copy link
Member Author

Choose a reason for hiding this comment

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

Added this comment:

"""Vulture allowlist.

Python names that we want Vulture to ignore need to be added to this file, see:

https://github.com/jendrikseipp/vulture/blob/main/README.md#whitelists
"""

Comment on lines 4 to 9
"""

# Authors: The MNE-Python contributors.
# License: BSD-3-Clause
# Copyright the MNE-Python contributors.
import glob
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI @drammock it would be easy enough now I think to make sure there is a newline here before code if we want it, too.

@larsoner
Copy link
Member Author

@drammock okay for you now or did you want me to make that change above about having a newline after the comments before any code?

@cbrnr
Copy link
Contributor

cbrnr commented Aug 14, 2024

Nice, I like this better than the SPDX thingy.

@drammock
Copy link
Member

@drammock okay for you now or did you want me to make that change above about having a newline after the comments before any code?

personally I like having the newline there. But if you make that change, please do it as [ci skip], there's no reason to waste resources on a whitespace change!

@larsoner
Copy link
Member Author

Okay pushed but I'll merge master in just to make sure we didn't miss any. Marking for merge when green -- I'll try to cut 1.9 tomorrow.

@larsoner larsoner enabled auto-merge (squash) August 14, 2024 16:47
@larsoner
Copy link
Member Author

... and pushed a tiny fix for https://github.com/mne-tools/mne-python/actions/runs/10379695427/job/28738303267 as I realized it hadn't run. Will hopefully work tonight, though! 🤞

@larsoner larsoner merged commit ee64eba into mne-tools:main Aug 14, 2024
@larsoner larsoner deleted the headers branch August 14, 2024 19:34
@cbrnr
Copy link
Contributor

cbrnr commented Aug 14, 2024

Thanks @larsoner!

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.

Simplify license headers in source files

4 participants