Supervised Reconstruction Dataset + bug fixes and formatting

[GabrielBG0]

No description provided.

[otavioon]

I suggest to remove the __getitem__ implementation, as it is equivalent to its parent class implementaion. If you wont want, I would suggest to change the return type from __getitem__ to Tuple[Any,Any], as inner type could not be infered yet (depends on readers' return type).

[otavioon]

The SimpleDataset already implements the equivaltent __getitem__code to this one.
Thus, you can omit this getitem implementation.
It worth noticing that we cannnot infer the return type (inside the tuple) yet, as it depends of the reader. The second element could be an int instead of np.ndarray, that represents the label, for instance.

[GabrielBG0]

The goal with this dataset is to be as specific as possible, hence the Transform Pipeline and the (exactly) two readers. The implementation is almost the same but it uses only one transform for both data points and it returns a known type as its output (the numpy array tuple), which is better for code downstream in the full pipeline process. Sure, as it is implemented, I can’t be sure what types are returned by the readers but if that’s a problem I would rather put a check to ensure it then to return Any.

[otavioon]

Hello @GabrielBG0. I did not see the class name, my bad.

1. I agree with a more specific implementation. However, I think this class could be a bit more generic. The name suggests that this dataset would only be used for Semantic Segmentation classes, which is not true. In fact, the same implementation here would be used for any other reconstruction task (predicting seismic attributes, seismic facies classification/segmentation, etc). Thus, any task that takes an input and has a target with the same shape as input should subclass this one. Therefore, maybe we can change its name to something like SupervisedReconstructionDataset. What do you think?
2. I agree with the typing hint, as it is a specific class. However, it is worth notice that, yes, this is the same behavior as in the base class. If you have only one Transform and several readers, the same transform will be applied to all data fetched from the readers (equivalent to the codehere). We can still rewrite this whole __getitem__ impementation to a simple super().__getitem__(index). This would reduce code duplication and unit test cases.

[GabrielBG0]

Sure, what do you think about it now?

[otavioon]

Hello @GabrielBG0. I did not see the class name, my bad.

1. I agree with a more specific implementation. However, I think this class could be a bit more generic. The name suggests that this dataset would only be used for Semantic Segmentation classes, which is not true. In fact, the same implementation here would be used for any other reconstruction task (predicting seismic attributes, seismic facies classification/segmentation, etc). Thus, any task that takes an input and has a target with the same shape as input should subclass this one. Therefore, maybe we can change its name to something like SupervisedReconstructionDataset. What do you think?
2. I agree with the typing hint, as it is a specific class. However, it is worth notice that, yes, this is the same behavior as in the base class. If you have only one Transform and several readers, the same transform will be applied to all data fetched from the readers (equivalent to the codehere). We can still rewrite this whole __getitem__ impementation to a simple super().__getitem__(index). This would reduce code duplication and unit test cases.

[otavioon]

LGTM! @GabrielBG0 I've fixed some typos in class' documentation and add some examples. Please, check if it is OK.