cytomining / pycytominer

Python package for processing image-based profiling data
https://pycytominer.readthedocs.io
BSD 3-Clause "New" or "Revised" License
76 stars 34 forks source link

[Enhancement] Create bug report functionality #365

Open gwaybio opened 8 months ago

gwaybio commented 8 months ago

in #320, @shntnu introduced a function, which @kenibrewer and I recommended be removed from the PR to be added separately later. This issue documents @shntnu 's progress in this effort:

Would it be possible to move the new create_bug_report_message function into a separate branch and pull request to be reviewed separately?

Makes sense, but instead, I'll just copy the code snippet here because I bet you'd want to do this more systematically later

```py def create_bug_report_message(error_detail, context): """ Constructs an error message with a bug report prompt. Parameters ---------- error_detail: str Description of the error. context: str A description of the context for the error. Returns ------- str Error message with a link to file a bug report. """ bug_report_url = "https://github.com/cytomining/pycytominer/issues" return ( f"{error_detail} This is likely a bug in `{context}`. " "Please help us improve our software by filing a bug report at " f"{bug_report_url}." ) def test_create_bug_report_message(): error_detail = "Division by zero error occurred." context = "calculate_statistics function" actual_message = create_bug_report_message(error_detail, context) # check that the strings `error_detail` and `context` are in the message assert error_detail in actual_message assert context in actual_message # check that the message contains a link to file a bug report assert "https://github.com/cytomining/pycytominer/issues" in actual_message ``` _Originally posted by @shntnu in https://github.com/cytomining/pycytominer/issues/320#issuecomment-1882081223_
shntnu commented 8 months ago

Thanks Greg!

For the record:

  1. I proposed it with hesitance because I feel there must already be some standard engineering pattern that addresses this
  2. It is probably better implemented as an exception, like the bot suggests below
import inspect

class BugReportError(Exception):
    def __init__(self, message):
        stack = inspect.stack()
        # The caller function is at index 1 (0 is current function - __init__)
        _, _, _, function_name, _, _ = stack[1]

        BUG_REPORT_URL = "https://github.com/cytomining/pycytominer/issues"
        bug_message = (
            f"This is likely a bug."
            "Please help us improve our software by filing a bug report at "
            f"{BUG_REPORT_URL}."
        )

        full_message = (
            f"BugReportError in {function_name}: {message}"
            f"\n\n{bug_message}"
            )

        super().__init__(full_message)
import pytest
import inspect
from your_module import BugReportError  # Replace 'your_module' with the actual module name

def test_bug_report_error_message():
    def dummy_function():
        raise BugReportError("Test error message")

    with pytest.raises(BugReportError) as exc_info:
        dummy_function()

    error_message = str(exc_info.value)
    assert "dummy_function" in error_message
    assert "Test error message" in error_message
    assert "https://github.com/cytomining/pycytominer/issues" in error_message

# Run the test with pytest from the command line
kenibrewer commented 7 months ago

I like the suggestion of using a dedicated error class too. I did some research and that's closer to standard practice.

I think it might be good to create an exceptions.py module for the different exception classes we want to support. And then put something like this in it:

class PycytominerException(Exception):
    BUG_REPORT_URL = "https://github.com/cytomining/pycytominer/issues"
    EXPERIMENTAL_FEATURE_ADDON = f"""
        This error occurred in a new/experimental feature of pycytominer and may be a bug. 
        Please help us improve our software by filing a bug report at {BUG_REPORT_URL}
        """
    def __init__(self, message: str,  experimental:bool = False)
        message = self.EXPERIMENTAL_FEATURE_ADDON + message if experimental else message
        super.__init__(message)

This would also address two concerns I had around the previous method.

1) Defining a strategy for when we want to take ownership of an error and explicitly request bug reports 2) The ability to switch off the bug report prompts without major refactoring of code.