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

[Enhancement] Create bug report functionality #365

Open
gwaybio opened this issue Jan 12, 2024 · 2 comments
Open

[Enhancement] Create bug report functionality #365

gwaybio opened this issue Jan 12, 2024 · 2 comments

Comments

@gwaybio
Copy link
Member

gwaybio commented Jan 12, 2024

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

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 #320 (comment)

@shntnu
Copy link
Member

shntnu commented Jan 13, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants