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

feat: add csv parsing for meshes and tutorial #638

Merged
merged 12 commits into from
Dec 21, 2022

Conversation

guenthermi
Copy link
Member

@guenthermi guenthermi commented Dec 14, 2022

Add csv parsing for meshes and tutorial


@github-actions github-actions bot added the area/testing This issue/PR affects testing label Dec 14, 2022
Copy link
Member

@bwanglzu bwanglzu left a comment

Choose a reason for hiding this comment

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

docs/notebooks/mesh_to_mesh.md Show resolved Hide resolved
docs/notebooks/mesh_to_mesh.md Outdated Show resolved Hide resolved
Copy link
Contributor

@LMMilliken LMMilliken left a comment

Choose a reason for hiding this comment

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

two small things

finetuner/data.py Outdated Show resolved Hide resolved
Comment on lines +248 to +249
if create_point_clouds and os.path.isfile(column):
doc.load_uri_to_point_cloud_tensor(point_cloud_size)
Copy link
Contributor

Choose a reason for hiding this comment

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

this document would be unusable if create_point_clouds is False but column is a local file right?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes more or less, but it is the same for images when you set convert_to_blob=False.

Copy link
Contributor

Choose a reason for hiding this comment

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

good point, maybe both of these cases should raise an error or warning

@bwanglzu
Copy link
Member

can you use a different font color? switch between the dark and light mode you'll see it
2109B84B-94CA-4FC9-BDC8-B79699002D6A


```python id="vDVkw65kkQcn"
!pip install 'finetuner[full]'
!pip install 'docarray[full]'
Copy link
Member

Choose a reason for hiding this comment

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

let's remove this from the tutorial

Copy link
Member

@bwanglzu bwanglzu left a comment

Choose a reason for hiding this comment

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

LGTM!, minor comment

@guenthermi
Copy link
Member Author

let's remove this from the tutorial

Oh yes, this is terrible. I can add a white backround surronding all the image of the meshes

@guenthermi guenthermi marked this pull request as ready for review December 20, 2022 15:44
CHANGELOG.md Outdated
Comment on lines 17 to 18
- Ass support for 3d meshes to `build_finetuning_dataset`. ([#638](https://github.com/jina-ai/finetuner/pull/638))

Copy link
Contributor

Choose a reason for hiding this comment

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

typo

Copy link
Contributor

@LMMilliken LMMilliken left a comment

Choose a reason for hiding this comment

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

One small thing

Copy link
Contributor

@LMMilliken LMMilliken left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link

📝 Docs are deployed on https://ft-docs-add-3d-mesh-example--jina-docs.netlify.app 🎉

@bwanglzu
Copy link
Member

is there an error in hit@k metric? seems it's not improving

@guenthermi guenthermi merged commit a527c87 into main Dec 21, 2022
@guenthermi guenthermi deleted the docs-add-3d-mesh-example branch December 21, 2022 09:03
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.

None yet

3 participants