Improve code style: fix imports and PEP 8 violations#52
Conversation
Co-authored-by: ZHZisZZ <[email protected]>
… and trailing whitespace Co-authored-by: ZHZisZZ <[email protected]>
|
revert the following changes: "Renamed ambiguous single-letter variable l → seq_len in trainer and evaluation code" |
Co-authored-by: ZHZisZZ <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves code consistency and adherence to Python style guidelines across 60+ files in the codebase while preserving all functionality. The changes focus on import organization and PEP 8 compliance.
Key changes:
- Applied isort with black profile to organize imports across the codebase (stdlib → third-party → local)
- Replaced bare
except:clauses withexcept Exception:for better exception handling practices - Fixed PEP 8 None comparisons (
!= None→is not None) - Removed trailing whitespace from documentation and example files
- Removed unused type hint imports that were not referenced in the code
Reviewed changes
Copilot reviewed 60 out of 66 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/tests/test_attention.py | Sorted imports: pytest moved after third-party imports |
| examples/llada/sft.py, pt.py, chat.py | Removed trailing whitespace; sorted imports (accelerate before transformers) |
| examples/editflow/*.py | Sorted imports alphabetically; removed trailing whitespace from comments |
| examples/dream/*.py | Sorted imports; removed trailing whitespace from documentation |
| examples/bert/*.py | Sorted imports; removed trailing whitespace |
| examples/a2d//.py | Sorted imports; removed trailing whitespace |
| dllm/utils/visualizers.py | Sorted imports from rich library alphabetically; sorted stdlib imports |
| dllm/utils/utils.py | Sorted imports: logging before os; alphabetized third-party imports |
| dllm/utils/models.py | Fixed bare except → except Exception; fixed None comparison to use is not None; sorted imports |
| dllm/utils/data.py | Sorted imports; alphabetized TYPE_CHECKING imports |
| dllm/utils/configs.py | Alphabetized local imports |
| dllm/utils/collators.py | Moved typing import to proper position with blank line separator |
| dllm/utils/chat.py | Moved typing imports after stdlib; removed extra blank lines |
| dllm/utils/init.py | Alphabetized import order |
| dllm/tools/*.py | Sorted imports alphabetically |
| dllm/pipelines/*/eval.py | Sorted imports: moved tqdm after lm_eval imports |
| dllm/pipelines/*/init.py | Alphabetized imports for consistency |
| dllm/pipelines/editflow/utils.py | Alphabetized typing imports |
| dllm/pipelines/editflow/trainer.py | Alphabetized typing imports; removed extra blank lines |
| dllm/pipelines/dream/sampler.py | Sorted imports: torch.distributions after torch.nn.functional |
| dllm/data/*.py | Removed unused type imports (Optional, List, Dict); sorted remaining imports |
| dllm/core/trainers/*.py | Moved typing imports to proper position after from future |
| dllm/core/schedulers/*.py | Removed unused type imports (Dict, Type); added blank line after future |
| dllm/core/samplers/*.py | Sorted base imports alphabetically; removed multi-line import formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .data import * | ||
| from .sampling import * | ||
| from .models import * | ||
| from .sampling import * |
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module dllm.utils.sampling does not define 'all'.
|
|
||
| if TYPE_CHECKING: | ||
| from dllm.utils.configs import ModelArguments, DataArguments, TrainingArguments | ||
| from dllm.utils.configs import DataArguments, ModelArguments, TrainingArguments |
There was a problem hiding this comment.
Import of 'ModelArguments' is not used.
Import of 'TrainingArguments' is not used.
|
|
||
| from dllm.utils.utils import disable_caching_allocator_warmup, print_main, load_peft | ||
| from dllm.utils.configs import ModelArguments, TrainingArguments | ||
| from dllm.utils.utils import disable_caching_allocator_warmup, load_peft, print_main |
There was a problem hiding this comment.
Import of 'disable_caching_allocator_warmup' is not used.
|
|
||
| import os | ||
| from dataclasses import dataclass, asdict | ||
| from dataclasses import asdict, dataclass |
There was a problem hiding this comment.
Import of 'asdict' is not used.
| from dataclasses import dataclass, asdict | ||
| from dataclasses import asdict, dataclass | ||
| from functools import partial | ||
| from pprint import pprint |
There was a problem hiding this comment.
Import of 'pprint' is not used.
| import logging | ||
| from contextlib import contextmanager | ||
| from dataclasses import dataclass, asdict | ||
| from dataclasses import asdict, dataclass |
There was a problem hiding this comment.
Import of 'dataclass' is not used.
| from typing import Any, Dict, List, Optional, Tuple, Text | ||
| from collections.abc import Callable | ||
| from dataclasses import dataclass | ||
| from typing import Any, Dict, List, Optional, Text, Tuple |
There was a problem hiding this comment.
Import of 'Dict' is not used.
Import of 'List' is not used.
Import of 'Tuple' is not used.
Import of 'Optional' is not used.
Import of 'Text' is not used.
| from typing import Any, Dict, List, Optional, Text, Tuple | |
| from typing import Any |
| import re | ||
| import unicodedata | ||
| from typing import Optional, List, Tuple, Annotated | ||
| from typing import Annotated, List, Optional, Tuple |
There was a problem hiding this comment.
Import of 'Tuple' is not used.
Import of 'Optional' is not used.
Import of 'Annotated' is not used.
| from typing import Optional, List, Tuple, Annotated | ||
| from typing import Annotated, List, Optional, Tuple | ||
|
|
||
| from PIL import Image, ImageDraw, ImageFont |
There was a problem hiding this comment.
Import of 'Image' is not used.
Import of 'ImageDraw' is not used.
Import of 'ImageFont' is not used.
| from abc import ABC, abstractmethod | ||
| from typing import Sequence, Optional | ||
| from dataclasses import dataclass | ||
| from typing import Optional, Sequence |
There was a problem hiding this comment.
Import of 'Sequence' is not used.
Reduces stylistic inconsistencies across the codebase while preserving all functionality. Models directories remain untouched per existing black configuration.
Import Organization
Optional,Dict,List,Type) from data loaders and schedulersPEP 8 Compliance
except:withexcept Exception:!= None→is not NoneOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.