Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes unused imports across multiple test files and code modules, while also making minor code formatting improvements to adhere to style guidelines.
- Removed numerous unused imports from test files and modules
- Fixed f-string usage by removing unnecessary f-prefixes where string interpolation wasn't used
- Updated version number from 0.6.27 to 0.6.28
- Improved code formatting and line wrapping for better readability
Reviewed Changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/* | Removed unused imports across multiple test files |
| mabel/version.py | Updated version number to 0.6.28 |
| mabel/utils/* | Improved code formatting and removed unnecessary f-strings |
| mabel/data/* | Enhanced code formatting and readability |
| mabel/adapters/* | Code style improvements and formatting |
| examples/Reading Data.ipynb | Removed unused Reader import |
| Makefile | Updated build configuration with better formatting |
Comments suppressed due to low confidence (1)
mabel/data/writers/writer.py:1
- Similar to the previous issue, these InvalidDataSetError exceptions are created but not raised. They should be prefixed with
raiseto actually throw the exceptions.
import datetime
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if "BACKOUT" in dataset: | ||
| InvalidDataSetError("BACKOUT is a reserved word and cannot be used in Dataset names") | ||
| InvalidDataSetError( | ||
| "BACKOUT is a reserved word and cannot be used in Dataset names" | ||
| ) |
There was a problem hiding this comment.
The InvalidDataSetError exception is created but not raised. This should be raise InvalidDataSetError(...) to actually throw the exception.
| for key in top.keys(): | ||
| key_type = {type(val).__name__ for val in top.collect_list(key) if val != None} | ||
| key_type = { | ||
| type(val).__name__ for val in top.collect_list(key) if val != None |
There was a problem hiding this comment.
Use is not None instead of != None for None comparisons to follow Python best practices.
| type(val).__name__ for val in top.collect_list(key) if val != None | |
| type(val).__name__ for val in top.collect_list(key) if val is not None |
| values = [ | ||
| item.get(column) for item in items if item.get(column) is not None | ||
| ] |
There was a problem hiding this comment.
The item.get(column) call is duplicated for each item. Consider storing the result in a variable to avoid the double lookup: value = item.get(column); if value is not None.
| values = [ | |
| item.get(column) for item in items if item.get(column) is not None | |
| ] | |
| values = [] | |
| for item in items: | |
| value = item.get(column) | |
| if value is not None: | |
| values.append(value) |
No description provided.