DOC reorganize converter docs #1268
Conversation
…organize_converters
…organize_converters
…organize_converters
rlundeen2
left a comment
rlundeen2
left a comment
There was a problem hiding this comment.
Approving - great work!
I feel like with both my comments we should reach consensus. You may change my mind about either, but if you agree and implement the changes we don't need to resync
| logger = logging.getLogger(__name__) | ||
| logger.setLevel(logging.DEBUG) |
There was a problem hiding this comment.
is this just not necessary in this instance or notebooks in general ?
| - file: code/converters/1_text_to_text_converters | ||
| - file: code/converters/2_audio_converters | ||
| - file: code/converters/3_image_converters | ||
| - file: code/converters/4_video_converters | ||
| - file: code/converters/5_file_converters | ||
| - file: code/converters/6_selectively_converting | ||
| - file: code/converters/7_human_converter |
There was a problem hiding this comment.
I like this organization a lot! this is much more streamlined. I have a slight preference to remove 7_human_converter so that human converter is just an example in the 1_text_to_text_converters notebook
There was a problem hiding this comment.
The interaction mode is VERY different, though, and it prevents us from running it in automation. Plus, I imagine with the GUI this will actually disappear.
| https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#webp-saving | ||
| """ | ||
|
|
||
| SUPPORTED_INPUT_TYPES = ("image_path", "url") |
There was a problem hiding this comment.
is url not image path ?
|
|
||
| converter_modalities.append((name, input_modalities, output_modalities)) | ||
|
|
||
| except (AttributeError, TypeError): |
There was a problem hiding this comment.
so if someone adds a new converter, but doesn't set the supported input/output types even if this should be set, we won't throw an error ? what's the case we wouldn't want these set ?
Co-authored-by: hannahwestra25 <hannahwestra@microsoft.com>
4 participants
Description
Bringing better order to converter docs by showing all converters at least once, sorting by I/O modalities.
This also meant adding a function to get all converters sorted by modality which is showcased in the notebook indexed with 0. For simplicity, the I/O modalities are now on a class constant and retrieved for the property getters
supported_input_typesandsupported_output_types.Tests and Documentation
Added test to make sure all converters (except base classes) are indeed shown in the converter docs there is a new unit test file.
Reran all the converter notebooks.