Add test cases for caption_images component and fixed bug in this com…

[satishjasthi]

This the first commit to add test cases to all existing components of fondant. This PR tries to resolve #301

[mrchtr]

Thanks @satishjasthi! The test case is already a good starting point to build a unit test fundament!

During a quick offline discussion with @GeorgesLorre about the general test layout we came up with an idea.
It would be great if we would have something like an AbstractComponentTest which can be used for all components.

The basic idea is to make the implementation of unit tests as easy as possible for the users. In the best case scenario, users would only need to define a dataframe containing their input dummy data, the expected output data, and the component arguments. A default class could be created to handle tasks such as invoking the component's transform method and comparing the expected output dataframe with the actual result.

Would you be up for drafting such a class and use it to test the caption_images component?

[satishjasthi]

Hey @mrchtr, Ya sure that sounds like a better idea, I would love to create such an abstract class for caption_images component.

[satishjasthi]

Thanks @satishjasthi! The test case is already a good starting point to build a unit test fundament!

During a quick offline discussion with @GeorgesLorre about the general test layout we came up with an idea. It would be great if we would have something like an AbstractComponentTest which can be used for all components.

The basic idea is to make the implementation of unit tests as easy as possible for the users. In the best case scenario, users would only need to define a dataframe containing their input dummy data, the expected output data, and the component arguments. A default class could be created to handle tasks such as invoking the component's transform method and comparing the expected output dataframe with the actual result.

Would you be up for drafting such a class and use it to test the caption_images component?

Hey @mrchtr, I have pushed commit for AbstractComponentTest class. My idea was to create a pytest class which can validate

• Whether a new component created has base component as one of the components defined in fondant.component.py
• And to validate whether this new component has all the methods and its respective attributes that are defined in its base class

This can be used to test all the components, expect the actually functionality of the components which varies from one another, so user can add test cases as methods to this class to incorporate validations for those custom component behaviours.
Please let me know your views on this. Thank you

[mrchtr]

Thanks for the update @satishjasthi! I didn't think about your test cases, they are interesting.
I think we can combine our ideas here. In my mind, I had a slightly different approach. I would see it more as a base class, with concrete component tests inheriting from it.

So basically to have something like this for the AbstractComponentTest:

class AbstractComponentTest(ABC):

    @abstractmethod
    def create_component(self):
        """
        This method should be implemented by concrete test classes to create the specific component
        that needs to be tested.
        """
        raise NotImplementedError()

    @abstractmethod
    def input_dataframe(self):
        """
        This method should be implemented by concrete test classes to create the specific input dataframe.
        """
        raise NotImplementedError()

    def setUp(self):
        """
        This method will be run before each test method.
        Add any common setup steps for your components here.
        """
        self.component = self.create_component()
        self.input_dataframe = self.create_input_dataframe()
        self.expected_output_dataframe = self.create_output_dataframe()

    def tearDown(self):
        """
        This method will be run after each test method.
        Add any common cleanup steps for your components here.
        """
        pass
  
    def test_transform(self):
        """
        Default test for the transform method.
        Tests if the transform method executes without errors.
        """
        transformed_data = self.component.transform(self.input_dataframe)
        # compare transformed_data with self.expected_output

Here, we could implement default test methods that would be easy to maintain in the future. If we add new functionality for the base components, we wouldn't have to edit each single unit test. I suggest starting with a test_transform method that calls the transform method of the component and checks if the transformed output matches the expected one.

For the concrete implementation we just have to inherit from the abstract one and implement the create_component, create_input_dataset and create_output_dataset methods.

Based on the class provided, you can modify the caption_image_component test by inheriting from the base class and implementing the initialisation of the component and the datasets.

So something like this:

class CaptionImageComponentTest(AbstractComponentTest):

     def create_component(self):
         # your implementation
     ...

I also would like to hear other opinions on this as well.

[satishjasthi]

Hi @mrchtr, Thanks for the detailed explanation, I like this idea as it can easily extend to all components easily. I'll create a base class in this manner and submit new commit.

[satishjasthi]

Hey @mrchtr, I have updated the test case for CaptionImage with a new base class and used it for testing this component. Please review it and let me know your views.

[mrchtr]

Thanks a lot again @satishjasthi! I left some smaller comments. Got the feeling we are on the right track.

[mrchtr]

Can we move this implementation to the super class, since it should be the same for most components?

[satishjasthi]

Yes we can, as this method has pretty much stand method of testing transform method for all classes.

[mrchtr]

I guess we can remove this method. I have added the tearDown just as en example and I think we don't need it here explicitly.

[satishjasthi]

Indeed, I felt the same but I retained it as its standard template in pytest testing with setup and teardown methods. I'll remove that.

[mrchtr]

Is it possible to remove this method as well? Ideally, the setUp of the super class is called.

[satishjasthi]

Yes we can, as the code within this method is again remains same across components

[mrchtr]

Later on, this class should be moved to fondant itself, so that users can reuse it.

[satishjasthi]

Sure, that makes sense

[satishjasthi]

@mrchtr, I have updated and pushed new commit with suggested changes, please review it.

[mrchtr]

Looks good to me. I have tested the code locally.
Added on super small nitpick.

I think we can merge it, and see it as a first blueprint for further component test. Thanks again!

[mrchtr]

Nitpick - pandas offers you already an assertion function:
pd.testing.assert_frame_equal(output,self.expected_output_data)

[satishjasthi]

@mrchtr , I have pushed code with this change. Shall I create test cases for remaining components in the same PR or a new one?