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.

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

Add ECD Descriptions #2897

Merged
merged 49 commits into from
Jan 12, 2023
Merged

Add ECD Descriptions #2897

merged 49 commits into from
Jan 12, 2023

Conversation

connor-mccorm
Copy link
Collaborator

@connor-mccorm connor-mccorm commented Jan 5, 2023

Adds specific descriptions for each encoder, combiner, and decoder. I added a short and a long description - short description for UI display, long description for in depth documentation. I have put these descriptions on the metadata so the type parameter under the schema classes now reference the metadata for those descriptions. Lastly, I have added the descriptions on the json_schema_mapping so that it can be accessed from the schema for each different encoder/combiner/decoder.

@connor-mccorm connor-mccorm changed the title Added encoder descriptions Add ECD Descriptions Jan 5, 2023
@connor-mccorm
Copy link
Collaborator Author

Actually going to make a small change here where I put the descriptions in the metadata so I can also maintain relevant links.

@github-actions
Copy link

github-actions bot commented Jan 5, 2023

Unit Test Results

         6 files  ±  0           6 suites  ±0   4h 20m 44s ⏱️ - 24m 59s
  3 866 tests +  3    3 792 ✔️ +  5    74 💤 ±  0  0  - 2 
11 598 runs  +76  11 376 ✔️ +65  222 💤 +13  0  - 2 

Results for commit 45d05b8. ± Comparison against base commit 7ebe3b8.

♻️ This comment has been updated with latest results.

@martindavis
Copy link
Contributor

Hey @connor-mccorm , nice work getting this into Ludwig so quickly! One callout: some of these descriptions seems really long and not particularly helpful in teaching people why they would choose a particular encoder/decoder/combiner. Can we add one sentence to the beginning of each description explaining when/why to use each option?

Copy link
Collaborator

@justinxzhao justinxzhao left a comment

Choose a reason for hiding this comment

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

LGTM, just some final nits/questions.

ludwig/schema/combiners/utils.py Show resolved Hide resolved
ludwig/schema/decoders/utils.py Show resolved Hide resolved
ludwig/schema/metadata/configs/combiners.yaml Outdated Show resolved Hide resolved
ludwig/schema/metadata/configs/combiners.yaml Outdated Show resolved Hide resolved
ludwig/schema/metadata/configs/combiners.yaml Outdated Show resolved Hide resolved
ludwig/schema/metadata/configs/combiners.yaml Outdated Show resolved Hide resolved
ludwig/schema/metadata/configs/decoders.yaml Outdated Show resolved Hide resolved
ludwig/schema/metadata/parameter_metadata.py Show resolved Hide resolved
@connor-mccorm connor-mccorm merged commit 28937a3 into master Jan 12, 2023
@connor-mccorm connor-mccorm deleted the descriptions branch January 12, 2023 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants