Update Models methods & properties (WIP) #93
Conversation
…ml5-website-v02-docsify into alan-methods&properties
✅ Deploy Preview for ml5-next-gen-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hi @alanvww , thanks for the amazing updates! One thing I noticed, can we include options that are shared by MoveNet and Blaz |
|
@QuinnHe got it! I will work on it now :) |
|
Quick update for the docs! Just confirmed with Peter, all |
|
Resolved conflicts for updated to main and also included #112 |
|
@alanvww some bodyseg really new PRs and Issues for your reference, the second one seems more urgent to be included: |
I can fix it quickly now. But I didn't realize NN's workload was so huge. I better skip Sound Classifier and focus on this. In the meantime, would you please also check the preview deployment of this PR to see the new content I added? I tried my best to match the source code, but it might still be missing something. |
Yes, I will go through them after I complete BodySeg other sections. |
Thanks Alan, will review soon! |
docs/reference/body-segmentation.md
Outdated
|
|
||
| [More info on options for BodyPix model.](https://github.com/tensorflow/tfjs-models/blob/master/body-segmentation/src/body_pix/README.md#create-a-detector) | ||
|
|
||
| - **callback(bodySegmentation, error)**: OPTIONAL. A function to run once the model has been loaded. Alternatively, call `ml5.bodySegmentation()` within the p5 `preload` function. | ||
| - **callback(bodySegmentation)**: OPTIONAL. A function to run once the model has been loaded. Alternatively, call `ml5.bodySegmentation()` within the p5 `preload` function. |
There was a problem hiding this comment.
I remembered we still have error, it's just we don't use it in example code. We could double check this after soft launch, feel free to create a Issue for this.
There was a problem hiding this comment.
Ok, I will consider putting it back with the ? indicating that it is optional. It is actually quite important for debugging.
docs/reference/sentiment.md
Outdated
|
|
||
| --- | ||
|
|
||
| #### .predict() | ||
| ### sentiment.predict() | ||
|
|
||
| > Given a number, will make magicSparkles |
There was a problem hiding this comment.
> Given a number, will make magicSparkles This does not make a lot sense to me? Maybe we can remove this? Or Alan you have any specific reasons for keeping it?
There was a problem hiding this comment.
I did this to be consistent with the methods in the other models page. But I also realize that NN has a completely different structure compared to other models. Should we use the .functionName() style for all the pages?
There was a problem hiding this comment.
I did this to be consistent with the methods in the other models page. But I also realize that NN has a completely different structure compared to other models. Should we use the
.functionName()style for all the pages?
Oh no I refer to > Given a number, will make magicSparkles this line, maybe we should remove this line?
There was a problem hiding this comment.
Ohh got it. At least for this one, we should remove it. But the same question still, in NN a lot of > are working as method description. Should I reformat that too? I might need to work on it tomorrow morning if that is ok :/
There was a problem hiding this comment.
Ohh got it. At least for this one, we should remove it. But the same question still, in NN a lot of
>are working as method description. Should I reformat that too? I might need to work on it tomorrow morning if that is ok :/
Yes, if time is permitted, let's remove > for NN as well. We can do a merge tomorrow morning around 10-11am ish before the presentation. If can't make it, it's fine, we can keep > for now.
There was a problem hiding this comment.
👌, that is feasible timing.
There was a problem hiding this comment.
For Sentiment, let's update "Properties" style to align with "Methods". We can remove --- divider, and remove >.
|
@alanvww , I've done reviewing the models you have right now! I have to say, this is AWESOME! Thank you so much for your work Alan!! |
Thanks Quinn! Thanks for the reviews. My schedule is too fragmented these days and sometimes I can't finish reading the codes right away, so I have to go over things again and again. I wish I didn't mess things up too much lol. |
|
Hey @alanvww , just let you know that I will start to work on SoundClassifier now, let's avoid duplicated work :) |
Haha as always you did awesome, no worries at all! |
All good! |
| - String: The type of model to use. Default: "MULTIPOSE_LIGHTNING". | ||
| - _enableSmoothing_ - Optional | ||
| - Boolean: Whether to smooth the pose landmarks across different input images to reduce jitter. Default: true. | ||
|
|
There was a problem hiding this comment.
We may need to add Options for MoveNet model only as well. See https://github.com/ml5js/ml5-next-gen/blob/main/src/BodyPose/index.js#L27-L32 for reference
There was a problem hiding this comment.
Thanks for point that out! It was in the previous commit but I accidentally removed it when solving conflict when updating. Will fix it now.
|
@QuinnHe @MOQN Last comment before bed. I just added Question about NN: The origin methods section of it is more like an instruction. The way I modify it right now is to make it match with other page's style more. But I think those contents are still very helpful for beginners. Maybe we should separate it from the methods and use it in the guide? |
Sound Classifier is done. #0649346 |
Thanks @alanvww for the work on One small thing - can we make sure the |
Got it. I am on my way to school now. Will fix that soon. |
|
Thanks @alanvww ! This looks great! Merged |
This PR is for updating methods and properties for each models. @ziyuan-linn Peter, thanks for the amazing works of the library and commenting everything! This update is based on the those content and also the latest examples in the collection. Feel free to review and leave any comments!
@QuinnHe @myrahsa
Progress