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

Scrubber sample code now handles "singular" tokens correctly #233

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

0dB
Copy link
Contributor

@0dB 0dB commented Aug 6, 2023

Scrubber sample code now handles tokens correctly that also appear by their own and not just in spans with other tokens.

When adding a token to the scrubber list like "two" that also exists as an individual term not just together with other tokens like "two sentences" the original code failed because the while loop consumed the whole span consisting of just one token. Fixed.

… their own.

When adding a token to the scrubber list like "two" that also exists as an individual term not just together with other tokens like "two sentences" the original code failed because the while loop consumed the whole span consisting of just one token. Fixed.
@ceteri
Copy link
Collaborator

ceteri commented Aug 7, 2023

@0dB ,

Could you make the following change in the .github/workflows/ci.yml file in your branch, then re-submit this PR?

A few lines need to be commented out:

name: CI

on: [push, pull_request, workflow_dispatch]

jobs:
#  pre-commit:
#    name: Run pre-commit
#    runs-on: ubuntu-latest
#    steps:
#      - uses: actions/checkout@v2
#      - uses: actions/setup-python@v2
#      - uses: pre-commit/action@v2.0.0

  test:
    name: Tests for Python ${{ matrix.python-version }}
    runs-on: ubuntu-latest
    strategy:
      matrix:
        python-version: ['3.7', '3.8', '3.9', '3.10']
      fail-fast: false
#    needs: pre-commit

    steps:
      - uses: actions/checkout@v2

      - name: Set up Python
        uses: actions/setup-python@v2
        with:
          python-version: ${{ matrix.python-version }}

      - name: Install dependencies
        run: |
          pip install -e .
          spacy download en_core_web_sm
          pip install pre-commit pytest

      - name: Run tests
        run: |
          pytest

GitHub currently is failing on the CI / Run pre-commit step, and we need to disable this step before a PR can be tested and merge.

And I want to be sure to give you credit for the PR in our Git history of the project.

@0dB
Copy link
Contributor Author

0dB commented Aug 7, 2023

Done, but I also see you are fighting with some issues; I'm afraid I am no help with those.

@ceteri
Copy link
Collaborator

ceteri commented Aug 7, 2023

Many thanks!
And no worries about the CI issues, we've got lots of devops experience here -- when GitHub is running correctly :)

FWIW, the CI actions built by GitHub for pre-commit hooks when through 3 versions; however, last year they archived that project and starting referring people to a different external project. GitHub doing something a bit strange with Py 3.11 when one attempts to use those archived actions. We just need to cut over to the newer ones.

Although we were blocked because once the CI actions on GitHub reach an error, there's really no way to back out without deleting or updating a PR.

@ceteri ceteri merged commit 42776b8 into DerwenAI:main Aug 7, 2023
4 checks passed
@0dB 0dB deleted the 0dB-patch-1 branch August 7, 2023 20:07
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.

2 participants