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

Fix: Error handling for forward() to only take one pandas dataframe #1382

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from

Conversation

duanduanzhou
Copy link

This pull request addresses Issue #1217 , focusing on enhancing error handling and input constraints within the forward() method. The key change involves enforcing a length constraint on io_signature to ensure forward() only accepts a single pandas DataFrame as input.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

👋 Hello @duanduanzhou, thanks for submitting a EVA DB PR 🙏 To allow your work to be integrated as seamlessly as possible, we advise you to:

  • ✅ Verify that your PR is up-to-date with georgia-tech-db/eva master branch. If your PR is behind you can update your code by clicking the 'Update branch' button or by running git pull and git merge master locally.
  • ✅ Verify that all EVA DB Continuous Integration (CI) checks are passing.
  • ✅ Reduce changes to the absolute minimum required for your bug fix or feature addition.

@duanduanzhou duanduanzhou changed the title Enhanced error handling and testing for forward() to only take one pa… Fix: Error handling for forward() to only take one pandas dataframe Nov 22, 2023
@@ -322,6 +322,22 @@ def test_should_raise_if_function_file_is_modified(self):
# with self.assertRaises(AssertionError):
execute_query_fetch_all(self.evadb, select_query)

def test_should_raise_error_for_multiple_dataframes(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this to the short integration tests.

from evadb.functions.decorators.io_descriptors.data_types import PandasDataframe


class FuzzDistance(AbstractFunction):
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do not need to create a separate file for this function under evadb/functions, since this is not a valid function.

We can directly define this function in the integration test file.

@xzdandy
Copy link
Collaborator

xzdandy commented Nov 28, 2023

This PR fixes #1217

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

Successfully merging this pull request may close these issues.

2 participants