Skip to content

Commit

Permalink
Simplify the code formatting script
Browse files Browse the repository at this point in the history
After cvat-ai#8611 and cvat-ai#8866, it's no longer necessary to apply the formatters to
each module separately. So don't!

In addition, update the black exclusion list in `pyproject.toml` to only contain
files that were _not_ listed in `format_python_code.sh`. This way, formatting
will automatically be enforced for all new files.

For a few of the files reformatting only creates a small diff, so don't even
bother excluding them.
  • Loading branch information
SpecLad committed Jan 8, 2025
1 parent b594b1c commit 173c53b
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 102 deletions.
20 changes: 11 additions & 9 deletions cvat/apps/engine/management/commands/syncperiodicjobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,19 @@
class Command(BaseCommand):
help = "Synchronize periodic jobs in Redis with the project configuration"

_PERIODIC_JOBS_KEY_PREFIX = 'cvat:utils:periodic-jobs:'
_PERIODIC_JOBS_KEY_PREFIX = "cvat:utils:periodic-jobs:"

def add_arguments(self, parser: ArgumentParser) -> None:
parser.add_argument('--clear', action='store_true', help='Remove jobs from Redis instead of updating them')
parser.add_argument(
"--clear", action="store_true", help="Remove jobs from Redis instead of updating them"
)

def handle(self, *args, **options):
configured_jobs = defaultdict(dict)

if not options["clear"]:
for job in settings.PERIODIC_RQ_JOBS:
configured_jobs[job['queue']][job['id']] = job
configured_jobs[job["queue"]][job["id"]] = job

for queue_name in settings.RQ_QUEUES:
self.stdout.write(f"Processing queue {queue_name}...")
Expand All @@ -34,7 +36,7 @@ def handle(self, *args, **options):
scheduler = django_rq.get_scheduler(queue_name, queue=queue)

stored_jobs_for_queue = {
member.decode('UTF-8') for member in queue.connection.smembers(periodic_jobs_key)
member.decode("UTF-8") for member in queue.connection.smembers(periodic_jobs_key)
}
configured_jobs_for_queue = configured_jobs[queue_name]

Expand All @@ -51,12 +53,12 @@ def handle(self, *args, **options):

# Add/update jobs from the configuration
for job_definition in configured_jobs_for_queue.values():
job_id = job_definition['id']
job_id = job_definition["id"]

if job := queue.fetch_job(job_id):
if (
job.func_name == job_definition['func']
and job.meta.get('cron_string') == job_definition['cron_string']
job.func_name == job_definition["func"]
and job.meta.get("cron_string") == job_definition["cron_string"]
):
self.stdout.write(f"Job {job_id} is unchanged")
queue.connection.sadd(periodic_jobs_key, job_id)
Expand All @@ -68,8 +70,8 @@ def handle(self, *args, **options):
self.stdout.write(f"Creating job {job_id}...")

scheduler.cron(
cron_string=job_definition['cron_string'],
func=job_definition['func'],
cron_string=job_definition["cron_string"],
func=job_definition["func"],
id=job_id,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
} if context == "organization" else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down Expand Up @@ -172,27 +176,26 @@ def is_valid(scope, context, ownership, privilege, membership, resource, same_or
if context == "sandbox" and same_org:
return False


return True


def gen_test_rego(name):
with open(f"{name}_test.gen.rego", "wt") as f:
f.write(f"package {name}\nimport rego.v1\n\n")
for scope, context, ownership, privilege, membership, same_org, in product(
SCOPES, CONTEXTS, OWNERSHIPS, GROUPS, ORG_ROLES, SAME_ORG,
for scope, context, ownership, privilege, membership, same_org in product(
SCOPES, CONTEXTS, OWNERSHIPS, GROUPS, ORG_ROLES, SAME_ORG
):
for resource in RESOURCES(scope):
if not is_valid(
scope, context, ownership, privilege, membership, resource, same_org,
scope, context, ownership, privilege, membership, resource, same_org
):
continue

data = get_data(
scope, context, ownership, privilege, membership, resource, same_org
)
test_name = get_name(
scope, context, ownership, privilege, membership, resource, same_org,
scope, context, ownership, privilege, membership, resource, same_org
)
result = eval_rule(scope, context, ownership, privilege, membership, data)
f.write(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
16 changes: 9 additions & 7 deletions cvat/apps/engine/rules/tests/generators/issues_test.gen.rego.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
16 changes: 9 additions & 7 deletions cvat/apps/engine/rules/tests/generators/jobs_test.gen.rego.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
16 changes: 9 additions & 7 deletions cvat/apps/engine/rules/tests/generators/server_test.gen.rego.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,15 @@ def get_data(scope, context, ownership, privilege, membership):
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
}

Expand Down
16 changes: 9 additions & 7 deletions cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,15 @@ def get_data(scope, context, ownership, privilege, membership, resource, same_or
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
17 changes: 10 additions & 7 deletions cvat/apps/engine/rules/tests/generators/users_test.gen.rego.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def RESOURCES(scope):
{"id": random.randrange(300, 400), "membership": {"role": role}} for role in ORG_ROLES
]


def eval_rule(scope, context, ownership, privilege, membership, data):
if privilege == "admin":
return True
Expand Down Expand Up @@ -81,13 +82,15 @@ def get_data(scope, context, ownership, privilege, membership, resource):
"scope": scope,
"auth": {
"user": {"id": random.randrange(0, 100), "privilege": privilege},
"organization": {
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None,
"organization": (
{
"id": random.randrange(100, 200),
"owner": {"id": random.randrange(200, 300)},
"user": {"role": membership},
}
if context == "organization"
else None
),
},
"resource": resource,
}
Expand Down
25 changes: 2 additions & 23 deletions dev/format_python_code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,5 @@ if ! ${ISORT} --version >/dev/null 2>&1; then
exit 1
fi

# The commands must be run on each module directory separately,
# otherwise tools confuse the "current" module
for paths in \
"cvat-sdk" \
"cvat-cli" \
"tests/python/" \
"cvat/apps/quality_control" \
"cvat/apps/analytics_report" \
"cvat/apps/engine/lazy_list.py" \
"cvat/apps/engine/background.py" \
"cvat/apps/engine/frame_provider.py" \
"cvat/apps/engine/cache.py" \
"cvat/apps/engine/default_settings.py" \
"cvat/apps/engine/field_validation.py" \
"cvat/apps/engine/model_utils.py" \
"cvat/apps/engine/task_validation.py" \
"cvat/apps/dataset_manager/tests/test_annotation.py" \
"cvat/apps/dataset_manager/tests/utils.py" \
"cvat/apps/events/signals.py" \
; do
${BLACK} -- ${paths}
${ISORT} -- ${paths}
done
${BLACK} .
${ISORT} --resolve-all-configs .
40 changes: 39 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,45 @@ line-length = 100
target-version = ['py39']
extend-exclude = """
# TODO: get rid of these
^/cvat/apps/(dataset_manager|engine)/
^/cvat/apps/(
dataset_manager/(
formats/
|tests/test_formats.py
|tests/test_rest_api_formats.py
|[^/]+.py
)
|engine/(
admin.py
|apps.py
|backup.py
|cloud_provider.py
|filters.py
|handlers.py
|location.py
|log.py
|media_extractors.py
|middleware.py
|migrations/
|mime_types.py
|mixins.py
|models.py
|pagination.py
|parsers.py
|permissions.py
|plugins.py
|renderers.py
|rq_job_handler.py
|schema.py
|serializers.py
|signals.py
|task.py
|tests/
|urls.py
|utils.py
|view_utils.py
|views.py
)
)
| ^/cvat/settings/
| ^/serverless/
| ^/utils/dataset_manifest/
Expand Down
4 changes: 1 addition & 3 deletions site/content/en/docs/contributing/coding-style.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,4 @@ for indentation of nested blocks and statements.

For Python, we use [Black](https://github.com/psf/black) and
[isort](https://pycqa.github.io/isort/) to enforce the coding style and autoformat files.
Currently, not all components implement formatting, the actual information about the enabled
components is available in the CI checks [here](https://github.com/cvat-ai/cvat/tree/develop/.github/workflows)
and in the formatting script at `dev/format_python_code.sh`.
You can use `dev/format_python_code.sh` to apply these formatters.

0 comments on commit 173c53b

Please sign in to comment.