polyscope icon indicating copy to clipboard operation
polyscope copied to clipboard

Appropriate Forum For PR Priorities?

Open weshoke opened this issue 4 years ago • 6 comments

There are a couple of PRs that are very little code, but quite useful: https://github.com/nmwsharp/polyscope/pull/107 https://github.com/nmwsharp/polyscope/pull/138

I've tried to figure out what the best way to get a YES/NO on these is. I'm unsure if they're just going to be ignored or if there are some issues they bring up that should be discussed.

I know it's a lot of work to manage open source projects. I'd like to be as helpful as possible and not add to the maintenance burden. How can I help move these forward?

Some background ... I've been using Polyscope from Python a lot the last few months and have built a number of extensions. I'd like to work the changes into master to the degree that it makes sense so that I don't have to constantly keep managing my fork as master moves forward. Any guidance as to how to participate is welcome. Thank you for the excellent library!

I'm using it for 5-axis CNC machining applications: Screen Shot 2021-09-22 at 10 04 26 AM

weshoke avatar Nov 10 '21 06:11 weshoke

Ha, just now in the last few hours I've been writing code related to #107! I was very confused when I got the notification with it tagged here, what a coincidence.

But anyway, I understand the frustration more broadly, I apologize for not giving feedback on those more quickly! Still learning to be a good maintainer as Polyscope has grown rapidly recently. My main challenge is that managing cpp + unit tests + docs + python bindings mean that even simple changes can easily take 2+ hours.

I very much appreciate your investment to share changes & improve Polyscope!

Generally, I'd say this is the best place to ask. Feel free to tag me in issues and call stuff to my attention if I'm not being responsive within a week or so :) If I'm not going to address something or am getting annoyed, I'll say so! (but I don't think that will be an issue).

Lastly, lots of the Polyscope codebase is not well-suited to development by folks other than me... if you hit pain points, or things that don't make sense, please feel free to ask general questions or suggest improvements.

nmwsharp avatar Nov 10 '21 06:11 nmwsharp

Woah, just saw your screenshot! Looks awesome! Would love for any relevant extensions to get integrated back.

nmwsharp avatar Nov 10 '21 06:11 nmwsharp

My main challenge is that managing cpp + unit tests + docs + python bindings mean that even simple changes can easily take 2+ hours

This makes a lot of sense. The documentation is a huge help, so syncing any changes with the docs is important and certainly adds to the amount of work. It's obvious now that you mention it. I'll try to include any documentation changes in PRs where it's relevant.

The size of the library is such that it's not super hard to wind one's way through it and see how it works. I haven't found much difficulty adding extensions so far.

The biggest challenge, since I'm using polyscope via Python, is figuring out how to add custom ImGui UIs. The PyImGui lib maintains its own ImGui state and I don't think there's a way to pass it a pointer to an existing ImGui instance. After trying a number of approaches like modifying py-imgui etc., I settled on embedding custom ImGui bindings within polyscope-py.

The biggest changes I've made thus far are:

  • ability to set the transform on a Structure (necessary for moving the 5-axis machine components around)
  • custom ImGui callback in Python with ImGui bindings in polyscope-py
  • an event callback in Python on frame render / keyboard / mouse events to make custom 3D interactions
  • API in Python for getting/setting the currently selected Structure
  • API in Python for getting registered structures

weshoke avatar Nov 10 '21 06:11 weshoke

Responding real quick regarding ImGui bindings & userCallback in python, will address other things later: I actually just added some of these last week because I needed them in my own work too!

They're currently in this branch https://github.com/nmwsharp/polyscope-py/tree/callback. I'm hoping to include it in the next version. If there's anything you'd like to add/change/try out it would be very welcome!

nmwsharp avatar Nov 10 '21 06:11 nmwsharp

Oh, cool. I have bindings for a large fraction of ImGui. I'll take a look at what you've done and see about making PRs there/

weshoke avatar Nov 10 '21 06:11 weshoke

Ha, just now in the last few hours I've been writing code related to #107! I was very confused when I got the notification with it tagged here, what a coincidence.

But anyway, I understand the frustration more broadly, I apologize for not giving feedback on those more quickly! Still learning to be a good maintainer as Polyscope has grown rapidly recently. My main challenge is that managing cpp + unit tests + docs + python bindings mean that even simple changes can easily take 2+ hours.

I very much appreciate your investment to share changes & improve Polyscope!

Generally, I'd say this is the best place to ask. Feel free to tag me in issues and call stuff to my attention if I'm not being responsive within a week or so :) If I'm not going to address something or am getting annoyed, I'll say so! (but I don't think that will be an issue).

Lastly, lots of the Polyscope codebase is not well-suited to development by folks other than me... if you hit pain points, or things that don't make sense, please feel free to ask general questions or suggest improvements.

Just some thoughts:

Since the lib is written in cpp, I don't think we need to update both cpp and python simultaneously (the libigl is also maintained in this way). You can add some to take care of the python bindings, which can be a few steps slower than the cpp (same for the doc), which also helps the cpp part to be stable to use.

From my own experience (might be biased), python users in general only use the lib without too much customization, and they would also expect it to be more stable.

xarthurx avatar Nov 10 '21 10:11 xarthurx