merge: #63 from ginger/standardise

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,4 @@
+- [ ] I have read the [section on commits and pull requests](https://github.com/NaturalHistoryMuseum/ckanext-attribution/blob/main/CONTRIBUTING.md#commits-and-pull-requests) in `CONTRIBUTING.md`
+
+
+Describe your changes, tagging relevant issues where possible.

.github/workflows/bump.yml

@@ -7,12 +7,12 @@ on:
 
 jobs:
   bump-version:
-    if: "!startsWith(github.event.head_commit.message, 'bump:')"
+    name: Bump version and create changelog
     runs-on: ubuntu-latest
-    name: "Bump version and create changelog"
+    if: "!startsWith(github.event.head_commit.message, 'bump:')"
     steps:
-      - name: Check out
-        uses: actions/checkout@v3
+      - name: Checkout source code
+        uses: actions/checkout@v4
         with:
           token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
           fetch-depth: 0

.github/workflows/pull-requests.yml

@@ -0,0 +1,27 @@
+name: Validate pull requests
+
+on:
+  pull_request:
+    types: [opened, edited, reopened, synchronize]
+
+jobs:
+  validate-commits:
+    name: Validate commit messages
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout source code
+        uses: actions/checkout@v4
+      - name: Check commit message format
+        uses: webiny/[email protected]
+        with:
+          allowed-commit-types: 'feat,fix,refactor,perf,docs,style,test,build,ci,chore,new,patch,revert,ui'
+  pre-commit:
+    name: Run pre-commit checks
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout source code
+        uses: actions/checkout@v4
+      - name: Setup Python
+        uses: actions/setup-python@v5
+      - name: Run pre-commit
+        uses: pre-commit/[email protected]

.github/workflows/pypi-publish.yml

@@ -10,17 +10,16 @@ permissions:
 
 jobs:
   deploy:
+    name: Deploy package to PyPI
     runs-on: ubuntu-latest
     steps:
-      - name: Check out
-        uses: actions/checkout@v3
+      - name: Checkout source code
+        uses: actions/checkout@v4
         with:
           token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
           fetch-depth: 0
       - name: Set up Python
-        uses: actions/setup-python@v3
-        with:
-          python-version: '3.x'
+        uses: actions/setup-python@5
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip

.github/workflows/sync.yml

@@ -7,11 +7,11 @@ on:
 
 jobs:
   sync-branches:
+    name: Sync dev and patch branches to latest commit
     runs-on: ubuntu-latest
-    name: "Sync dev and patch branches to latest commit"
     steps:
-      - name: Check out
-        uses: actions/checkout@v3
+      - name: Checkout source code
+        uses: actions/checkout@v4
         with:
           token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
           fetch-depth: 0

.github/workflows/main.yml → .github/workflows/tests.yml

@@ -3,18 +3,18 @@ name: Tests
 on:
   push:
   workflow_dispatch:
+  pull_request:
+    types: [opened, edited, reopened, synchronize]
 
 jobs:
   test:
+    name: Run tests
     runs-on: ubuntu-latest
-
     steps:
       - name: Checkout source code
-        uses: actions/checkout@v3
-
+        uses: actions/checkout@v4
       - name: Build images
         run: docker compose build
-
       - name: Run tests
         env:
           COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}

.pre-commit-config.yaml

@@ -1,33 +1,34 @@
 exclude: /(vendor|dist)/
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.3.0
+    rev: v5.0.0
     hooks:
       - id: check-merge-conflict
       - id: detect-private-key
       - id: end-of-file-fixer
       - id: name-tests-test
-        args: ["--pytest-test-first"]
+        args: ['--pytest-test-first']
         exclude: ^tests/helpers/
       - id: trailing-whitespace
   - repo: https://github.com/commitizen-tools/commitizen
-    rev: v2.37.0
+    rev: v3.30.0
     hooks:
       - id: commitizen
-        additional_dependencies: ["cz-nhm"]
-  - repo: https://github.com/psf/black
-    rev: 22.10.0
+        additional_dependencies: ['cz-nhm']
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.7.1
     hooks:
-      - id: black
+      - id: ruff
+        args: [ '--fix', '--select', 'I', '--select', 'F401', '--fix-only' ]
+      - id: ruff-format
   - repo: https://github.com/PyCQA/docformatter
-    rev: v1.5.0
+    rev: eb1df34
     hooks:
       - id: docformatter
-        # these can't be pulled directly from the config atm, not sure why
-        args: ["-i", "--wrap-summaries=88", "--wrap-descriptions=88",
-               "--pre-summary-newline", "--make-summary-multi-line"]
+        args: [ '-i', '--config', './pyproject.toml' ]
+        additional_dependencies: ['tomli']
   - repo: https://github.com/pre-commit/mirrors-prettier
-    rev: v3.0.0-alpha.4
+    rev: v4.0.0-alpha.8
     hooks:
       - id: prettier
         types_or: [ javascript, vue, less, sass, scss, css ]

CONTRIBUTING.md

@@ -41,7 +41,7 @@ This extension and [several others](https://github.com/search?q=topic:ckan+org:N
 
 The current core team consists of:
 - Josh ([@jrdh](https://github.com/jrdh)) - Technical Lead
-- Ginger ([@alycejenni](https://github.com/alycejenni)) - Software Engineer
+- Ginger ([@alycejenni](https://github.com/alycejenni)) - Senior Software Engineer
 
 
 ## Questions
@@ -81,7 +81,10 @@ The process is generally as follows:
 3. Make your changes, and commit often; each commit should only contain one change. See below for specifics on how to word your commits.
 4. Push your changes back to your fork.
 5. [Open a pull request](https://docs.github.com/en/get-started/quickstart/contributing-to-projects#making-a-pull-request) in this repository, with the base branch set to **dev** and the compare branch set to your new branch. Provide a summary of your changes in the description.
-6. If the automatic tests fail (these may take a while), please go back to your code and try to make them pass. You may have to update the tests themselves. You don't have to close the pull request while you're doing this; it'll update as you add further commits.
+6. There are several automated checks that will run when you open the pull request. Try to make all of them pass. If you do not at least _attempt_ to make them pass, we will not merge your pull request.
+   1. Tests. Update them so that they pass, if necessary. New tests are always welcome in any pull request, but if you have added a new feature that has decreased the coverage, new tests are required.
+   2. Commit format validation. If you have not followed the conventional commits format for one or more of your commits, this will fail.
+   3. Code format validation. If you have not formatted your code correctly (using Ruff, docformatter, and/or Prettier), this will fail.
 7. Wait for feedback from one of the core maintainers. If it's been a week or so and we haven't responded, we may not have seen it. You can find other places to contact us in [SUPPORT.md](./.github/SUPPORT.md).
 
 ### Commits
@@ -142,9 +145,9 @@ cz c
 
 ##### pre-commit
 
-pre-commit is a tool that runs a variety of checks and modifications before a commit is made. You can check the [.pre-commit-config.yaml](./.pre-commit-config.yaml) file to see eaxtly what it's currently configured to do for this repository, but of particular note:
+pre-commit is a tool that runs a variety of checks and modifications before a commit is made. You can check the [.pre-commit-config.yaml](./.pre-commit-config.yaml) file to see exactly what it's currently configured to do for this repository, but of particular note:
 
-- reformats Python code with [Black](https://github.com/psf/black)
+- reformats Python code with [Ruff](https://docs.astral.sh/ruff)
 - reformats JavaScript and stylesheets with [Prettier](https://prettier.io)
 - reformats docstrings with [docformatter](https://github.com/PyCQA/docformatter)
 - checks your commit message is correcly formatted
@@ -161,15 +164,15 @@ pre-commit run
 
 Don't forget to stage any modifications that it makes! Once it runs without failing, then you can make your commit.
 
-Something to remember is that empty docstrings will cause conflicts between Black and docformatter and the checks will fail repeatedly - so don't leave your docstrings empty!
+Something to remember is that empty docstrings will cause conflicts between Ruff and docformatter and the checks will fail repeatedly - so don't leave your docstrings empty!
 
 ### Code changes and style guide
 
-We generally use external style guides and tools to help us maintain standardised code. Black and Prettier will be run with pre-commit.
+We generally use external style guides and tools to help us maintain standardised code. Ruff and Prettier will be run with pre-commit.
 
 #### Python
 
-We follow the [Black style](https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html), with the notable exception that we use single quotes.
+We use [Ruff](https://docs.astral.sh/ruff) to format our code, using defaults for everything except quote style (we use single quotes).
 
 We also _mostly_ use [CKAN's style](http://docs.ckan.org/en/latest/contributing/python.html), with the following exceptions:
 - prefer `f''` strings over `.format()`
@@ -178,7 +181,7 @@ We also _mostly_ use [CKAN's style](http://docs.ckan.org/en/latest/contributing/
 
 #### JavaScript and stylesheets (CSS, LESS, etc)
 
-We use [Prettier](https://prettier.io) to format these files.
+We use [Prettier](https://prettier.io) to format these files. As with Ruff, we use defaults for everything except quote style (we use single quotes).
 
 #### Accessibility
 

README.md

@@ -94,6 +94,8 @@ Path variables used below:
 
 ```shell
 pip install ckanext-attribution
+# to use the CLI as well:
+pip install ckanext-attribution[cli]
 ```
 
 ## Installing from source
@@ -112,6 +114,8 @@ pip install ckanext-attribution
 3. Install via pip:
    ```shell
    pip install $INSTALL_FOLDER/src/ckanext-attribution
+   # to use the cli as well:
+   pip install $INSTALL_FOLDER/src/ckanext-attribution[cli]
    ```
 
 ### Installing in editable mode
@@ -352,6 +356,8 @@ toolkit.get_action('agent_external_read')({}, data_dict)
 
 ## Commands
 
+**NB**: you will have to install the optional `[cli]` packages to use several of these commands.
+
 ### `initdb`
 ```shell
 ckan -c $CONFIG_FILE attribution initdb

ckanext/attribution/commands/cli.py

@@ -8,6 +8,9 @@
 from ckan.model import Session
 from ckan.model.package_extra import PackageExtra
 from ckan.plugins import toolkit
+from fuzzywuzzy import fuzz, process
+from sqlalchemy import and_, or_
+
 from ckanext.attribution.commands import migration
 from ckanext.attribution.logic.actions.helpers import get_author_string
 from ckanext.attribution.model import (
@@ -19,14 +22,12 @@
     relationships,
 )
 from ckanext.attribution.model.crud import (
-    AgentQuery,
-    PackageQuery,
-    PackageContributionActivityQuery,
     AgentAffiliationQuery,
     AgentContributionActivityQuery,
+    AgentQuery,
+    PackageContributionActivityQuery,
+    PackageQuery,
 )
-from fuzzywuzzy import process, fuzz
-from sqlalchemy import and_, or_
 
 
 def get_commands():
@@ -235,7 +236,7 @@ def merge_agents(q, match_threshold):
 @click.option(
     '--limit', help='Process n packages at a time (best for testing/debugging).'
 )
-@click.option('--dry-run', help='Don\'t save anything to the database.', is_flag=True)
+@click.option('--dry-run', help="Don't save anything to the database.", is_flag=True)
 @click.option(
     '--search-api/--no-search-api',
     help='Search external APIs (e.g. ORCID) for details.',

ckanext/attribution/commands/migration/__init__.py

@@ -1,4 +1,4 @@
-from .parser import Parser
-from .common import multi_choice, rgx
-from .combiner import Combiner
 from .api_search import APISearch
+from .combiner import Combiner
+from .common import multi_choice, rgx
+from .parser import Parser

ckanext/attribution/commands/migration/api_search.py

@@ -5,12 +5,19 @@
 # Created by the Natural History Museum in London, UK
 
 import click
+
 from ckanext.attribution.lib.orcid_api import OrcidApi
 from ckanext.attribution.lib.ror_api import RorApi
-from prompt_toolkit import prompt
-from nameparser import HumanName
 
-from .common import multi_choice
+try:
+    from nameparser import HumanName
+    from prompt_toolkit import prompt
+
+    cli_installed = True
+except ImportError:
+    cli_installed = False
+
+from .common import check_installed, multi_choice
 
 
 class APISearch(object):
@@ -45,6 +52,8 @@ def _search_api(self, contrib, lookup, api_name, result_format):
         :param result_format: display format of each result, e.g. "{name} ({location})"
         :return: None if not found, dict for matching result if found/selected
         """
+        check_installed(cli_installed)
+
         aff = '; '.join(contrib.get('affiliations', []))
         api = self.api[api_name]
         display_name = lookup.pop('display_name')
@@ -53,7 +62,7 @@ def _search_api(self, contrib, lookup, api_name, result_format):
                 f'Do any of these {api_name} results match "{display_name}" ({aff})?'
             )
             click.echo(f'\nSearching {api_name} for "{display_name}"...')
-            results = api.search(**lookup).get(u'records', [])
+            results = api.search(**lookup).get('records', [])
             if len(results) > 0:
                 i = multi_choice(
                     question,
@@ -94,6 +103,8 @@ def _search_api(self, contrib, lookup, api_name, result_format):
         return update_dict
 
     def _manual_edit(self, contrib, api_name):
+        check_installed(cli_installed)
+
         if click.confirm('Enter ID manually?'):
             api = self.api[api_name]
             _id = click.prompt(f'{api_name} ID', show_default=False).strip()

ckanext/attribution/commands/migration/combiner.py

@@ -8,13 +8,16 @@
 import re
 
 import click
-from ckanext.attribution.lib.orcid_api import OrcidApi
-from ckanext.attribution.lib.ror_api import RorApi
 from fuzzywuzzy import process
-from prompt_toolkit import prompt
-from unidecode import unidecode
 
-from .common import multi_choice
+try:
+    from unidecode import unidecode
+
+    cli_installed = True
+except ImportError:
+    cli_installed = False
+
+from .common import check_installed, multi_choice
 
 
 class Combiner(object):
@@ -30,7 +33,8 @@ def separate(self, group):
         """
         Ensure that the automated grouping is correct.
 
-        :param group: a list of ParsedSegment instances that are probably the same contributor
+        :param group: a list of ParsedSegment instances that are probably the same
+            contributor
         :return: a list of lists of ParsedSegments
         """
         all_names = sorted(
@@ -92,8 +96,10 @@ def combine_person_names(self, names):
         """
         Uses a list of HumanNames to determine the longest possible name for a person.
 
-        :return: a dict of family_name, given_names (includes middle names), and key (i.e. a sort/display name)
+        :return: a dict of family_name, given_names (includes middle names), and key
+            (i.e. a sort/display name)
         """
+        check_installed(cli_installed)
 
         def _filter_diacritics(name_list):
             filtered = [n for n in name_list if unidecode(n) != n]