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

Picking API and picking tutorial #233

Merged
merged 25 commits into from
Jul 1, 2020
Merged

Conversation

Garyfallidis
Copy link
Contributor

@Garyfallidis Garyfallidis commented Apr 7, 2020

advanced_picking

This PR introduces a new class called PickingManager .
This class helps with getting information from actors in the 3D world.
You can receive vertex indices, face indices, world coordinate positions and actor ids.

Also very importantly this class shows how to directly access and update the vertices and colors of objects. Have in mind that there could be many objects in one actor.

@Nibba2018, @filipinascimento, @lenixlobo, @mlraglin, @nasimanousheh

This PR resolves issue #228

@pep8speaks
Copy link

pep8speaks commented Apr 7, 2020

Hello @Garyfallidis! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-01 01:14:42 UTC

@codecov
Copy link

codecov bot commented Apr 7, 2020

Codecov Report

Merging #233 into master will decrease coverage by 0.19%.
The diff coverage is 62.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
- Coverage   88.56%   88.36%   -0.20%     
==========================================
  Files          18       19       +1     
  Lines        4469     4504      +35     
  Branches      571      580       +9     
==========================================
+ Hits         3958     3980      +22     
- Misses        367      372       +5     
- Partials      144      152       +8     
Impacted Files Coverage Δ
fury/utils.py 85.90% <20.00%> (-0.89%) ⬇️
fury/pick.py 70.00% <70.00%> (ø)

@Garyfallidis Garyfallidis changed the title Picking API and picking tutorial WIP: Picking API and picking tutorial Apr 8, 2020
@skoudoro
Copy link
Contributor

Hi @Garyfallidis,

When you finish with this PR, Can you rebase? it is hard to see your changes.

Thanks!

@skoudoro skoudoro added this to the v0.6.0 milestone Jun 25, 2020
@Garyfallidis
Copy link
Contributor Author

Yes of course.

@Garyfallidis Garyfallidis changed the title WIP: Picking API and picking tutorial Picking API and picking tutorial Jun 26, 2020
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 @Garyfallidis,

Really nice tutorial and useful functionality. See below some of my comments.

docs/tutorials/01_introductory/viz_picking.py Outdated Show resolved Hide resolved
docs/tutorials/01_introductory/viz_picking.py Outdated Show resolved Hide resolved
docs/tutorials/01_introductory/viz_picking.py Outdated Show resolved Hide resolved
docs/tutorials/01_introductory/viz_picking.py Show resolved Hide resolved
docs/tutorials/01_introductory/viz_picking.py Show resolved Hide resolved
docs/tutorials/01_introductory/viz_picking.py Show resolved Hide resolved
fury/pick.py Outdated Show resolved Hide resolved
fury/pick.py Outdated Show resolved Hide resolved
fury/pick.py Outdated Show resolved Hide resolved
fury/pick.py Show resolved Hide resolved
@Garyfallidis
Copy link
Contributor Author

Garyfallidis commented Jun 30, 2020

Addressed most of the comments. The only thing did not address is adding functionality for writing the buffers using pre-existing functions. This will need another PR and some extra thinking to do from what we offer here.

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 @Garyfallidis,

Can you please address the 2 last comments below. We need to fix CI's error before merging this PR.

docs/tutorials/01_introductory/viz_picking.py Outdated Show resolved Hide resolved
docs/tutorials/01_introductory/viz_picking.py Outdated Show resolved Hide resolved
@skoudoro
Copy link
Contributor

skoudoro commented Jul 1, 2020

Thank you @Garyfallidis, CI's errors are not related so merging.

@skoudoro skoudoro merged commit 5db0a6d into fury-gl:master Jul 1, 2020
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.

3 participants