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

DOC: GSoC Robin Final Report #929

Merged
merged 8 commits into from
Aug 26, 2024
Merged

Conversation

robinroy03
Copy link
Member

@robinroy03 robinroy03 commented Aug 21, 2024

Final report. The remaining blog posts will be linked when the PRs are merged.

Preview
image

@skoudoro
Copy link
Contributor

pip install -U numpydoc should fix this

@deka27
Copy link
Contributor

deka27 commented Aug 23, 2024


Amazing work here. Really inspiring tbh. Grateful to have you as a teammate. Also, keep in touch. .✌️


Regarding the code for the post -

I don't see any particular issue while rendering. I do get some warnings though.

image

I believe you have utilized the here target name too many time, might want to make it specific. It won't give an issue while rendering in my opinion, but making it specific will give cleaner code.

image

And the remaining blog post links, I assume will be added after the PR is merged for the posts.

For the timeline table, you can shorten url using something like this

`Blog 1 <https://fury.gl/latest/posts/2024/2024-06-06-week-1-robin.html>`__

But again, this is optional. It just looks cleaner.

Copy link
Contributor

@deka27 deka27 left a comment

Choose a reason for hiding this comment

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

Approving. As i don't see any technical issue.

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

hi @robinroy03 ,

overall looks good. Just make more explicit where is your work/repo for all elements.

it is a. bit hidden. otherwise, see the comments below

docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
@skoudoro
Copy link
Contributor

@WassCodeur is working hard to remove all documentation warnings so please, fix those warnings (it is almost done):

image

@skoudoro
Copy link
Contributor

@m-agour, @itellaetxe and @WassCodeur, please review. Thanks

Copy link
Member

@WassCodeur WassCodeur left a comment

Choose a reason for hiding this comment

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

Apart from that you've done a great job, and I've learned a lot from you, thanks for sharing.

Copy link

@itellaetxe itellaetxe left a comment

Choose a reason for hiding this comment

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

Amazing work, Robin. Again, this sounds like wizardy to me and I think it is a super cool project using cutting-edge tech. You found a very clever way to use LLMs. I wish I could do something like this!

On the other hand, I read through it carefully and it looks very neat and concise. The explanations are well structured, and the figures help understand what you are trying to say. I really like the figure that explains the architecture.

Again, super inspiring project. Keep it up.

As for the review, I +1'd some comments from the other reviewers, so you do not miss them. Other than those, I could not find anything that would need to be changed. LGTM, Approving.

docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@m-agour m-agour left a comment

Choose a reason for hiding this comment

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

Hi @robinroy03, Please look at my suggestions below. Thanks!

docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
docs/source/posts/2024/2024-08-21-final-report-robin.rst Outdated Show resolved Hide resolved
@robinroy03 robinroy03 requested a review from skoudoro August 25, 2024 15:12
Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Good job @robinroy03! merging

@skoudoro skoudoro merged commit 75f75ec into fury-gl:master Aug 26, 2024
17 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants