Conversation
…nto 138-feature-citation-reminder
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
why not Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
🤖 Code Formatting Reminder Hello there! 👋 It looks like the code in this pull request might benefit from some formatting improvements. |
|
/format |
|
🤖 I will now format your code with black. Check the status here. |
…n/preprocessing into 138-feature-citation-reminder
There was a problem hiding this comment.
Pull Request Overview
This PR implements a citation reminder feature for the brainles-preprocessing package. The reminder displays a message encouraging users to cite the package when using it in their research, controlled by an environment variable with opt-out capability.
- Adds a citation reminder decorator that displays a formatted message to users
- Integrates the reminder into the BasePreprocessor initialization
- Updates README with comprehensive citation information including BibTeX format
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| brainles_preprocessing/utils/citation_reminder.py | New module implementing the citation reminder decorator with environment variable control |
| brainles_preprocessing/preprocessor/preprocessor.py | Applies the citation reminder decorator to BasePreprocessor.init method |
| README.md | Adds citation section with paper reference and BibTeX entry |
| justify="center", | ||
| ) | ||
| console.rule() | ||
| console.line() |
There was a problem hiding this comment.
[nitpick] The console.line() call adds unnecessary whitespace. Consider removing it or making it conditional based on user preference, as it may clutter the output when the reminder is shown frequently.
| console.line() | |
| if os.environ.get("BRAINLES_PREPROCESSING_EXTRA_LINE", "false").lower() == "true": | |
| console.line() |
| if ( | ||
| os.environ.get("BRAINLES_PREPROCESSING_CITATION_REMINDER", "true").lower() | ||
| == "true" | ||
| ): |
There was a problem hiding this comment.
The environment variable is checked on every function call. Consider caching this value at module level or checking it only once to avoid repeated environment variable lookups.
| if ( | |
| os.environ.get("BRAINLES_PREPROCESSING_CITATION_REMINDER", "true").lower() | |
| == "true" | |
| ): | |
| if CITATION_REMINDER_ENABLED: |
| os.environ.get("BRAINLES_PREPROCESSING_CITATION_REMINDER", "true").lower() | ||
| == "true" | ||
| ): | ||
| console = Console() |
There was a problem hiding this comment.
Creating a new Console instance on every function call is inefficient. Consider creating a module-level Console instance to reuse across calls.
| console = Console() |
review after #121