Skip to content

#835 replace test copyright headers with ruff rule CPY#836

Open
Jared Drayton (mo-jareddrayton) wants to merge 6 commits intomainfrom
835-replace-test_copyright_headers-with-ruff-rule-cpy
Open

#835 replace test copyright headers with ruff rule CPY#836
Jared Drayton (mo-jareddrayton) wants to merge 6 commits intomainfrom
835-replace-test_copyright_headers-with-ruff-rule-cpy

Conversation

@mo-jareddrayton
Copy link
Collaborator

Closes #835

As noted by Ed (@mo-gill) the existing test_coding_standards.py doesn't actually test coding standards anymore and only checks for the copyright/license header. This PR replaces those tests in cdds and mip_convert with the ruff rule CPY (see https://docs.astral.sh/ruff/rules/#flake8-copyright-cpy)

A couple other changes include the removal of some spurious shebang lines and enabling the preview mode on ruff for the CPY rule.

Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM and makes this test a lot easier.

@mo-gill
Copy link
Collaborator

I take it that if we go ahead with this change we're just going to have to be aware from now on that it doesn't check non python files as opposed to the previous copyright checker which checked all of them?

@mo-jareddrayton
Copy link
Collaborator Author

I'm not sure if you saw our discussion on teams about this Matthew Mizielinski (@matthew-mizielinski)?

If not, the tldr is that this rule will only check .py files. I don't know how big of an issue you consider this?

@matthew-mizielinski
Copy link
Collaborator

If not, the tldr is that this rule will only check .py files. I don't know how big of an issue you consider this?

The other files we have are .cfg and .json (no headers), and the odd .cylc, .conf in the workflows. I'm less worried about these, but perhaps we should do both for now (I have more confidence in ruff than the unit test we have).

@mo-jareddrayton
Copy link
Collaborator Author

Jared Drayton (mo-jareddrayton) commented Mar 23, 2026

If not, the tldr is that this rule will only check .py files. I don't know how big of an issue you consider this?

The other files we have are .cfg and .json (no headers), and the odd .cylc, .conf in the workflows. I'm less worried about these, but perhaps we should do both for now (I have more confidence in ruff than the unit test we have).

I'm not even sure what files are being checked other than .py after looking at the list of excludes.

['egg-info', 'EGG-INFO', 'dist', '.pyc', 'doctrees', 'html', 'cfg', 'clyc',
'pylintrc', 'TAGS', 'json', 'todel', 'nfsc', 'txt', 'ini', 'conf', 'workflows']
)

I'll put it back in for now but at least we're more likely to pick up copyright issues during development with ruff check.

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.

Replace test_copyright_headers with ruff rule CPY

3 participants