Skip to content

FIX: Dataset passed to every single check #1156

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

unichronic
Copy link
Contributor

@unichronic unichronic commented Apr 8, 2025

This fixes #1080

Changes:

Refactored the code to remove the boilerplate of passing the dataset to every check method while maintaining the same functionality. The dataset is now accessible via the instance variable self.ds in all check methods.

@benjwadams
Copy link
Contributor

benjwadams commented Apr 10, 2025

Nice start. Unfortunately all checker subclasses use the (self, ds) function parameters as the calling convention as well. For example
https://github.com/ioos/compliance-checker/blob/main/compliance_checker/cf/cf_1_6.py#L633 along with all the other check_* methods in that module as well as others.

There's no easy way to do this without breaking other potential third party checkers. Including @ocefpaf for input here. I think at some level we'll need to keep the ds for now but refer to something like self.ds within the methods, possibly using some magic in the methods to check the value of ds and include a deprecation warning once per checker run with checkers that still use the old calling convention.

@ocefpaf
Copy link
Member

ocefpaf commented Apr 10, 2025

Indeed a deprecation warning and some testing against existing plugins would be nice. I do know that we have at least two plugins that are not in our CIs https://github.com/euro-cordex/cc-plugin-cc6 and https://github.com/uw-farlab/cc-plugin-og

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.

Dataset passed to every single check
3 participants