Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address Feedback + Autogen Docstring #44

Merged
merged 13 commits into from
Jan 31, 2025
Merged

Address Feedback + Autogen Docstring #44

merged 13 commits into from
Jan 31, 2025

Conversation

Farhan-Faisal
Copy link
Collaborator

@Farhan-Faisal Farhan-Faisal commented Jan 31, 2025

Peer Feedback 1 <htang085>.

  • Test Edge Cases: Add tests for invalid inputs (e.g., missing API keys or malformed files).
    - We added documentation for this. Integrating OpenAI itself was challenging, given the course load. We would make the tests for this more rigorous in future iterations.

Peer Feedback 2 <nhantien>:

  • I see you have a TODO to add OpenAI autogeneration, which is very creative, and it's also do-able with a combination of OpenAI python client + let the user set an environment variable for their API key.
    - Implemented

Peer Feedback 3 <Farhan-Faisal>
PEP8 Guidelines:

  • In func_dto.py and generate_docstring_template, the function headers are too long. Consider making them multiline to adhere to PEP8.
    • Headers broken down into multiline

Refactoring:

  • In func_dto.py, there is unnecessary use of list comprehension.
  • Instead of string parsing in func_dto.py, consider using inspect on the callable to retrieve type information directly.

Documentation:

  • In func_dto.py, class and method docstrings are missing.
  • In the README, the output type of read_user_function is incorrect— it should be FunctionDTO.
  • In the read_user_function docstring, the output type is also incorrect— it should be FunctionDTO.
  • Include instructions for running tests with a coverage report in the README.
  • You might need people to set up OpenAI API key in their environments. This should be indicated in the README in future iterations.

Tests:

  • Tests for read_user_function are failing. It seems like the type in the documentation is 'int', whereas it should be <class 'int'>.
  • Similar problem exists in test_fml_doc_gen
  • Docstring Completion (auto_generate):

Autogen:

  • It would be great if docstring completion were working! Looking forward to that.

- Implemented explicit type checking for function input
- Reformatted code to align with PEP8 standards
- Updated docstrings for clarity and consistency

Example Usage:
>>> def example_func(a, b):
...     return a + b
...
>>> sigDTO = read_user_function(example_func)
- Updated example Jupyter notebook to demonstrate the usage of  generate_docstring_template wih auto_generate
- Added step-by-step instructions for setting up OpenAI API key
- Included example function calls and expected outputs
- Added dotenv and open ai dependency to toml file
- Updated example notebook to demonstrate the usage of auto_generate
- Added step-by-step instructions for setting up OpenAI API key
- Included example function calls and expected outputs
- Added dotenv and openai dependencies to toml
Added `fill_docstring_with_ai` function to automatically generate detailed
docstrings using OpenAI's GPT-4. This function takes a docstring template
and function source as inputs and returns a completed docstring.

- Loads API key using `dotenv`
- Uses OpenAI's API for docstring completion
- Implements error handling for missing inputs and API failures
…) with error handling

- Added `auto_generate` option to `generate_docstring_template` to automatically generate docstrings using OpenAI's API.
- Implemented error handling for missing function input (`ValueError`).
- Ensured PEP 8 compliance with proper indentation, spacing, and docstring format.
- Added comprehensive unit tests for `read_user_function`, covering:
  - Functions with and without parameters
  - Functions with type hints, default values, and return types
  - Functions using `Optional` and `None` type annotations
  - Complex return types such as `list[int]`
  - Functions executing return statements
  - Error handling for invalid input

- Future improvement: Add checks for `src` attribute inside the returned object.
…_with_ai)

- Added missing test cases to cover all code paths in `fill_docstring_with_ai`
- Mocked API calls to verify correct request formation and error handling
- Ensured exceptions are raised for invalid inputs and API failures
- Included credit for auto-generated test cases
Added a section to the README detailing how to run tests using pytest and measure code coverage with poetry.
@Farhan-Faisal Farhan-Faisal self-assigned this Jan 31, 2025
Copy link

codecov bot commented Jan 31, 2025

Codecov Report

Attention: Patch coverage is 95.34884% with 2 lines in your changes missing coverage. Please review.

Project coverage is 93.61%. Comparing base (9e9893d) to head (b3c9713).

Files with missing lines Patch % Lines
src/fml_doc_gen/fml_doc_gen.py 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
- Coverage   94.20%   93.61%   -0.59%     
==========================================
  Files           6        7       +1     
  Lines          69       94      +25     
==========================================
+ Hits           65       88      +23     
- Misses          4        6       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Farhan-Faisal Farhan-Faisal added documentation Improvements or additions to documentation tests feature enhancement New feature or request labels Jan 31, 2025
@Farhan-Faisal Farhan-Faisal added this to the Milestone 4 (LAST ONE!) milestone Jan 31, 2025
@Farhan-Faisal Farhan-Faisal marked this pull request as ready for review January 31, 2025 01:02
@Farhan-Faisal Farhan-Faisal changed the title Autogen Address Feedback + Autogen Docstring Jan 31, 2025
Copy link
Collaborator

@mikem2m mikem2m left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -38,12 +38,16 @@ def square(base: int, pow: int) -> int:
print(generate_docstring_template(square, output_file=None))
```

## OpenAI Dependency:

`fml_doc_gen` requires an OpenAI API key to generate docstrings. You can set your API key using the `OPENAI_API_KEY` environment variable. Check our our documentation [here](https://readthedocs.org/projects/fml-doc-gen/badge/?version=latest) for more information.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: check *out our

@mikem2m mikem2m merged commit 8258e05 into main Jan 31, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request feature tests
Projects
Development

Successfully merging this pull request may close these issues.

4.7: Peer Review Feedback Improvements 4.5: Milestone 2 Feedback Improvements
2 participants