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.

Sign up for GitHub

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

Jump to bottom

Use cleaner field names in reusable components #679

Merged
merged 3 commits into from
Nov 28, 2023
Merged

Use cleaner field names in reusable components #679

merged 3 commits into from
Nov 28, 2023

Conversation

RobbeSneyders
Copy link
Member

@RobbeSneyders RobbeSneyders commented Nov 27, 2023 •
edited
Loading

This PR cleans up the field names in the reusable components. They were just concatenated when first migrating away from the subsets.

I tested all components with tests, fixed the outdated tests, and standardized the test directory structure. Each tests directory now has a pytest.ini so the PYTHONPATH is set correctly both inside and outside of docker, the test_requirements.txt was moved into the tests directory, and the Dockerfile was updated accordingly.

PhilippeMoussalli
PhilippeMoussalli reviewed Nov 28, 2023
View reviewed changes
components/language_filter/Dockerfile Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

language_filter -> filter_text_language

PhilippeMoussalli
PhilippeMoussalli reviewed Nov 28, 2023
View reviewed changes
Copy link
Contributor

@PhilippeMoussalli PhilippeMoussalli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks Robbe!

Left some suggestion on changing the name for a few components. I think we should generally stick to the convention action_modality for consistency

components/image_cropping/README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

crop_images

components/image_resolution_extraction/README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

extract_image_resolution

@RobbeSneyders
Copy link
Member Author

Updated the component names.

@PhilippeMoussalli PhilippeMoussalli merged commit 197ac59 into main Nov 28, 2023
6 checks passed
@PhilippeMoussalli PhilippeMoussalli deleted the feature/component-field-names branch November 28, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PhilippeMoussalli PhilippeMoussalli PhilippeMoussalli approved these changes

@GeorgesLorre GeorgesLorre Awaiting requested review from GeorgesLorre

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RobbeSneyders @PhilippeMoussalli