Expand Sequence list

[us]

Sequence gets expanded, for better readability.

Related issue #689

[Conchylicultor]

Thank you for fixing this. This fix seems a little hacky though.
I would prefer to use an more explicit check:

if isinstance(feature, FeaturesDict) or (
    isinstance(feature, Sequence) and isinstance(feature.feature, FeaturesDict)
):

[us]

I think for every complicated structure we need to new line. I thought of using {} to break lines. And all keys feature shown with {}.

[us]

At first I did like you, but I chose to write more general code but I can go back if you think code style should more understandable and readable.

[Conchylicultor]

The problem with if hasattr(v, 'keys'): is that if another FeatureConnector implement a .key attribute, the code will crash or behave in some unexpected way. I agree that your solution is shorter, but seems more hacky.

Alternatively, would it be possible to modify the FeatureDict.__repr__ to be displayed in multiple lines, such as print(builder.info.feature) is also displayed properly.

print(builder.info.features)
FeaturesDict({
    'label': ClassLabel(shape=(), dtype=tf.int64, num_classes=10),
    'image': Image(shape=(28, 28, 1), dtype=tf.uint8),
})

instead of currently:

FeaturesDict({'label': ClassLabel(shape=(), dtype=tf.int64, num_classes=10), 'image': Image(shape=(28, 28, 1), dtype=tf.uint8)})

Then you could update the usage in DatasetInfo.__repr__ and _pprint_features_dict in the doc wouldn't be required anymore.

[us]

@Conchylicultor I find an alternative solution please check it.

[Conchylicultor]

Thanks, this looks much better.

[Conchylicultor]

Instead of calling pprint_features_dict and have the kwargs hack which seems hacky, I feel it would be nicer to have FeaturesDict simply calling repr() recursively on its child.

Something like:

def __repr__(self):
  lines = ['{}({'.format(type(self).__name__)]
  for key, feature in sorted(list(features_dict.items())):
    all_sub_lines = '\'{}\': {},'.format(key, feature)
    lines.extend('    ' + l for l in all_sub_lines.split('\n'))  # Add indentation to all childs
  lines.append('})')
  return lines.join('\n')

This is more generic as this would works with any feature which is using multiple line. And it avoid having to test insinstance(, FeatureDict) or hasattr.
What do you think ?

[us]

I'm agree with you but now we don't use the pprint_features_dict func.

[Conchylicultor]

Great, thank you. Looks good. I'm merging it now.
Yes, pprint_features_dict isn't used anymore, so I'll remove it internally before merging.

Also, in features_dict.py, be careful not to import the full API import tensorflow_datasets as tfds, as it creates circular dependency. Instead individual modules should be imported. I'm fixing this internally before merging it too.