Skip to content

Conversation

@jvachier
Copy link
Owner

@jvachier jvachier commented May 15, 2025

Describe your changes

Provide a clear and concise description of the changes made in this pull request. Include any relevant context or background information.

  • What is the purpose of this pull request?
  • What problem does it solve?
  • What functionality does it add or improve?

Issue ticket number and link

Type of Change

Check the type of change your pull request introduces:

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (code improvements without changing functionality)

Checklist before requesting a review

Before submitting your pull request, ensure the following:

  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added or updated relevant documentation (if applicable).
  • My changes do not introduce any new warnings or errors.
  • I have checked for security vulnerabilities in the code.
  • I have ensured backward compatibility (if applicable).

@jvachier jvachier self-assigned this May 15, 2025
@jvachier jvachier requested a review from Copilot May 15, 2025 18:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request improves code clarity and maintainability by adding type annotations and enhancing docstrings across test suites and application modules. Key changes include:

  • Comprehensive type annotations for function signatures and return types in tests, processors, and model components.
  • Updated docstrings to clearly document function arguments and return types.
  • Refinements in the Makefile commands to include additional source directories for style checks.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_transformer_model.py Added type annotations and enhanced docstrings for test functions.
tests/test_data_processor.py Updated fixture and test functions with explicit type annotations.
src/translation_french_english.py Type annotations added to function parameters and return types.
src/sentiment_analysis.py Added return type annotation to main().
src/modules/transformer_components.py Added type annotations for evaluate_bleu function parameters.
src/modules/speech_to_text.py Added type annotations for functions to clarify return types.
src/modules/optuna_transformer.py Updated function signatures with proper type hints.
src/modules/model_bert_other.py Removed legacy code, streamlining the module.
src/modules/data_processor.py Added type annotations for improved clarity in data processing.
src/modules/data_preprocess_nltk.py Commented unused import for clarity.
app/voice_to_text_app.py Added type annotations to callback functions.
Makefile Adjusted file paths in ruff commands to target updated source directories.

@jvachier jvachier requested a review from Copilot May 15, 2025 18:23
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors the codebase by adding comprehensive type hints and enhancing docstrings, thereby improving code clarity and maintainability. Key changes include:

  • Addition of explicit type annotations and return types in tests, modules, and application code.
  • Renaming and updating of function signatures (e.g. renaming test_translation to translation_test).
  • Updating linting commands in the Makefile to cover new directories.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_transformer_model.py Added type hints to fixtures and test functions for clearer API expectations.
tests/test_data_processor.py Introduced return type annotations and improved docstrings for fixture functions.
src/translation_french_english.py Updated function signatures and renamed test_translation to translation_test.
src/sentiment_analysis.py Added an explicit return type for the main() function.
src/modules/transformer_components.py Added type hints to the evaluate_bleu function.
src/modules/speech_to_text.py Added return type annotations to several methods.
src/modules/optuna_transformer.py Updated function signatures with explicit type annotations.
src/modules/model_bert_other.py Removed deprecated/unused code to improve maintainability.
src/modules/data_processor.py Added type hints for function parameters and return values.
src/modules/data_preprocess_nltk.py Cleaned up import statements by commenting out unused imports.
app/voice_to_text_app.py Improved type annotations for the update_output callback function.
Makefile Updated ruff commands to check, fix, and format new directories.
Comments suppressed due to low confidence (2)

src/translation_french_english.py:137

  • The translation_test function uses 'np.argmax' but there is no import for numpy. Please add 'import numpy as np' at the top of the file to ensure this function works as expected.
sampled_token_index = np.argmax(predictions[0, i, :])

app/voice_to_text_app.py:110

  • [nitpick] The return type annotation for update_output is very generic. Consider replacing 'Any' with more specific types to improve code readability and maintainability.
def update_output(start_n_clicks: int, stop_n_clicks: int, recording_state: bool) -> Tuple[Any, Any, Any, Any, bool]:

@jvachier jvachier merged commit 4494bd0 into main May 15, 2025
1 check passed
@jvachier jvachier deleted the jv/uniformise_docstring_w_types branch May 15, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants