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

Making Ludwig and HuggingFace case insensitive #1090

Merged

Conversation

hershd23
Copy link
Contributor

Lowercasing function_type and target string before matching so as to make it case insensitive

modified:   evadb/binder/statement_binder.py
modified:   evadb/executor/create_function_executor.py

@xzdandy @gaurav274 do we want a test for checking lowercase as well? If so exactly what should it check?

Although #1071 discusses only Ludwig, I saw the same behaviour in the case of HuggingFace as well and made that case insensitive as well

…make it case insensitive

	modified:   evadb/binder/statement_binder.py
	modified:   evadb/executor/create_function_executor.py
@gaurav274
Copy link
Member

Fix the linter, and we should be good to go!

@xzdandy
Copy link
Collaborator

xzdandy commented Sep 11, 2023

Just some thoughts: maybe a generic until function that does case insensitive string equality check in https://github.com/georgia-tech-db/evadb/blob/staging/evadb/utils/generic_utils.py can improve clarity and simplify the code base.

@hershd23
Copy link
Contributor Author

Makes sense @xzdandy

	modified:   evadb/binder/statement_binder.py
	modified:   evadb/executor/create_function_executor.py
	modified:   evadb/utils/generic_utils.py
@hershd23
Copy link
Contributor Author

Facing a couple of weird issues here

Unit tests failing where I see all test cases as passed. I do see some skipped however

SKIPPED [3] test/unit_tests/test_eva_cmd_client.py: unconditional skip
SKIPPED [1] test/unit_tests/executor/test_plan_executor.py:225: disk_based_storage_deprecated
SKIPPED [1] test/unit_tests/parser/test_parser.py:154: Skip parser exception handling testcase, moved to binder
SKIPPED [1] test/unit_tests/server/test_configuration_file.py:26: Run only if Ray is enabledUNIT TEST CODE: --|120|-- FAILURE

Exited with code exit status 120

Lint tests also failed, I don't know how, I ran black evadb from the project root itself, the black version in the CI and my local are the same. Any suggestions @xzdandy @gaurav274 ?

I'll try syncing to staging and try again

@xzdandy
Copy link
Collaborator

xzdandy commented Sep 12, 2023

Facing a couple of weird issues here

Unit tests failing where I see all test cases as passed. I do see some skipped however

SKIPPED [3] test/unit_tests/test_eva_cmd_client.py: unconditional skip
SKIPPED [1] test/unit_tests/executor/test_plan_executor.py:225: disk_based_storage_deprecated
SKIPPED [1] test/unit_tests/parser/test_parser.py:154: Skip parser exception handling testcase, moved to binder
SKIPPED [1] test/unit_tests/server/test_configuration_file.py:26: Run only if Ray is enabledUNIT TEST CODE: --|120|-- FAILURE

Exited with code exit status 120

Lint tests also failed, I don't know how, I ran black evadb from the project root itself, the black version in the CI and my local are the same. Any suggestions @xzdandy @gaurav274 ?

I'll try syncing to staging and try again

Thanks for introducing string_comparison_case_insensitive!. It will be better if we can have some unit tests both for the utility function and the places that we use string_comparison_case_insensitive.

I have seen similar failures recently. And it seems that the unit tests are flaky now. Created issue #1098. I rerun the test, and now they are all good.

@gaurav274 gaurav274 merged commit e48729f into georgia-tech-db:staging Sep 12, 2023
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

Successfully merging this pull request may close these issues.

3 participants