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

Code for automatically updating license headers #2028

Merged
merged 14 commits into from
Nov 9, 2022

Conversation

stes
Copy link
Collaborator

@stes stes commented Oct 29, 2022

More updates:

  • As discussed, reverted the actual changes to the license headers for now
  • Added a new tools directory for future developer tools (like pre-commit functions, etc.)

Some updates:

It is not possible to configure various license statements directly in NOTICE.yml. This is also useful in general to have an overview over third partly licenses applicable to the code.

For updating license headers in the whole repository, simply run

python update_license_headers.py

now. Some TODOs:

  • Before merging, we need to remove the old docstring license headers. I left them in this PR to more easily compare if the correct licenses was applied everywhere. I can do that once I got that approval.
  • It should be decided where to put the update_license_headers.py helper script. Afaik there is not "developer tools" directory right now. The options I can think of are: (1) leave it in the repo root (2) put it in .github or (3) add it to deeplabcut.utils. Opinions?

Original PR description

Prior to this PR, the license headers in the codebase had two issues:

  • They were not standardized, and the version information was outdated ("DeepLabCut2.0", etc.)
  • They were added to the modules __doc__ variable because
  • Potentially maintainers used some automated tool for this, but the correct license header to use was not added to the repo

This PR proposes the following changes:

  • Use license headers that are started by # instead of tripe quotes ("""). This avoids a clash between the module docstring and makes it easier to write API docs on module level.
  • Update the contribution guide with instructions of how to automatically update the license headers (see below)
  • Add license headers to all files in tests/ and examples/
  • Updated the header to "DeepLabCut2.0-2.3 Toolbox" (the version info could optionally also be automatically inserted, if desired) DeepLabCut Toolbox
  • License header can now be configured in .copyright.tmpl file

The headers can always be updated by running (this can be automated and e.g. be added to a script that also triggers code formatting w/ black, etc):

find deeplabcut tests examples -iname '*.py' -type f \
  -exec licenseheaders -t .copyright.tmpl -f {} \;
licenseheaders -t .copyright.tmpl -f dlc.py

TODO (for maintainers):

  • Please check if the info in .copyright.tmpl is accurate/should be updated
  • update non-dlc copy right headers accordingly (see review comments)

@stes stes marked this pull request as ready for review October 29, 2022 15:26
Copy link
Collaborator Author

@stes stes left a comment

Choose a reason for hiding this comment

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

identified some incorrectly updated headers

deeplabcut/pose_estimation_tensorflow/config.py Outdated Show resolved Hide resolved
deeplabcut/post_processing/analyze_skeleton.py Outdated Show resolved Hide resolved
deeplabcut/utils/make_labeled_video.py Outdated Show resolved Hide resolved
deeplabcut/utils/video_processor.py Outdated Show resolved Hide resolved
examples/testscript_3d.py Outdated Show resolved Hide resolved
examples/testscript_openfielddata.py Outdated Show resolved Hide resolved
@stes stes marked this pull request as draft October 29, 2022 19:01
@stes stes force-pushed the stes/update-license-headers branch 3 times, most recently from 6cacf46 to bc865f1 Compare November 6, 2022 17:16
@stes
Copy link
Collaborator Author

stes commented Nov 6, 2022

@AlexEMG pls see my changes:

It is not possible to configure various license statements directly in NOTICE.yml. This is also useful in general to have an overview over third partly licenses applicable to the code.

For updating license headers in the whole repository, simply run

python update_license_headers.py

now. Some TODOs:

  • Before merging, we need to remove the old docstring license headers. I left them in this PR to more easily compare if the correct licenses was applied everywhere. I can do that once I got that approval.
  • It should be decided where to put the update_license_headers.py helper script. Afaik there is not "developer tools" directory right now. The options I can think of are: (1) leave it in the repo root (2) put it in .github or (3) add it to deeplabcut.utils. Opinions?

@stes stes marked this pull request as ready for review November 6, 2022 17:19
@MMathisLab
Copy link
Member

"It should be decided where to put the update_license_headers.py helper script. Afaik there is not "developer tools" directory right now. The options I can think of are: (1) leave it in the repo root (2) put it in .github or (3) add it to deeplabcut.utils. Opinions?"

  • we could make a dev tooling dir?

@AlexEMG AlexEMG changed the title Automatically update license headers Code for automatically updating license headers Nov 8, 2022
@stes stes force-pushed the stes/update-license-headers branch from 983cdc4 to f605b7a Compare November 8, 2022 23:02
@stes stes requested review from AlexEMG and MMathisLab and removed request for AlexEMG November 8, 2022 23:10
@stes stes requested review from AlexEMG and MMathisLab and removed request for MMathisLab and AlexEMG November 8, 2022 23:11
@stes
Copy link
Collaborator Author

stes commented Nov 8, 2022

More updates:

  • As discussed, reverted the actual changes to the license headers for now
  • Added a new tools directory for future developer tools (like pre-commit functions, etc.)

@AlexEMG @MMathisLab

Copy link
Member

@AlexEMG AlexEMG left a comment

Choose a reason for hiding this comment

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

Thanks, @stes!

@AlexEMG AlexEMG merged commit 3e51125 into DeepLabCut:master Nov 9, 2022
jonahpearl pushed a commit to jonahpearl/DeepLabCut that referenced this pull request Nov 27, 2022
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