ucam-department-of-psychiatry / camcops

Cambridge Cognitive and Psychiatric Test Kit (CamCOPS)
Other
12 stars 8 forks source link

Linting #279

Closed RudolfCardinal closed 1 year ago

RudolfCardinal commented 1 year ago

I have probably made a Git mess. This branch was for ACE-III versions (e.g. mini-ACE, remote edition) but possibly that has all been merged; what's left over below looks just like linter changes. OK to merge/delete?

martinburchell commented 1 year ago

The changes look fine. I see there is also a new list_tasks command.

It was wondering if we might adopt https://github.com/asottile/yesqa, which detects redundant #noqa comments. I tried to run the latest released version from pypi but was suspicious of the number of unused imports it detected. One for further investigation maybe.

It looks like alembic thinks there is a missing migration file and the python tests are broken.

RudolfCardinal commented 1 year ago

Do you understand the failure, @martinburchell ? It's failing on missing-migration-check.yml with the error test -z server/camcops_server/alembic/versions/0081_test.py but that is from a later branch (ISAAQ-10). Does this mean it's OK just to merge in?

martinburchell commented 1 year ago

Do you understand the failure, @martinburchell ? It's failing on missing-migration-check.yml with the error test -z server/camcops_server/alembic/versions/0081_test.py but that is from a later branch (ISAAQ-10). Does this mean it's OK just to merge in?

Yes and no. It's a bit cryptic. This check tells you that Alembic thinks something has changed in the database and there needs to be a new migration file. Buried in the log output are these lines:

 2023-03-19 18:37:10.313 env_py:INFO: upgrade_ops:
<alembic.operations.ops.UpgradeOps object at 0x7f422aa7bd60>
    <alembic.operations.ops.ModifyTableOps object at 0x7f4206aefbe0> for table _export_recipients
        <alembic.operations.ops.AlterColumnOp object at 0x7f421a9e7340> for column _export_recipients.file_patient_spec_if_anonymous
        <alembic.operations.ops.AlterColumnOp object at 0x7f421a9e7130> for column _export_recipients.hl7_debug_treat_diverted_as_sent

I pulled the branch and ran the create_database_migration.py script and Alembic thinks two of the comments for _export_recipients have changed. But if you look at the contents of the created file, the comments haven't changed at all. I wonder if having the lines split by Black is confusing things somewhere. I can have a look at it on Tuesday.

martinburchell commented 1 year ago

@RudolfCardinal actually I can see the problem. There were some rogue commas introduced into the comment strings as part of the noqa changes, which turned strings into tuples. I've pushed a change which should fix both this and the python test failure.

RudolfCardinal commented 1 year ago

Oops -- thank you very much!