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

Implemented model cursor for visual feedback #760 #781

Closed
wants to merge 10 commits into from

Conversation

Subh231004
Copy link

@Subh231004 Subh231004 commented Jun 19, 2024

Implemented model cursor for visual feedback
/claim #760

What kind of change does this PR introduce?

Summary

Checklist

  • My code follows the style guidelines of OpenAdapt
  • I have performed a self-review of my code
  • If applicable, I have added tests to prove my fix is functional/effective
  • I have linted my code locally prior to submission
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
  • New and existing unit tests pass locally with my changes

How can your code be run and tested?

Other information

Implemented model cursor for visual feedback

Co-Authored-By: Richard Abrich <774615+abrichr@users.noreply.github.com>
@abrichr
Copy link
Member

abrichr commented Jun 19, 2024

Good start! It might be easier to copy and paste the contents of vanilla.py and start from there. It's not clear to me what is the right abstraction for strategies that build on it.

@abrichr
Copy link
Member

abrichr commented Jun 19, 2024

I think probably we want to use a methodology similar to VisualReplayStrategy (i.e. segmentation + description) to generate initial cursor target candidates, then use the model cursor feedback to let it self-correct.

@Subh231004
Copy link
Author

@abrichr I've made some changes in the files please review them and let me know if something more needs to be done

@abrichr
Copy link
Member

abrichr commented Jun 20, 2024

@Subh231004 your code is unfortunately quite disorganized and difficult to read. I have enabled a build so that you can see the errors that will be reported by our style checkers. Please address those and I will be happy to take a closer look.

@Subh231004 Subh231004 closed this Jun 21, 2024
@Subh231004 Subh231004 reopened this Jun 21, 2024
@Subh231004
Copy link
Author

@abrichr can u please review the code I've tried to make it readable

@ayewo
Copy link

ayewo commented Jun 22, 2024

@abrichr

Just a quick FYI that similar low-effort PRs have been opened in other projects12 that end up getting closed.

I only noticed because I got quite a few nonsensical GitHub notifications from Subh231004 here3 that have since been deleted by the maintainers.

Footnotes

  1. https://github.com/langchain4j/langchain4j/pull/1316

  2. https://github.com/gyroflow/gyroflow/pull/847

  3. https://github.com/langchain4j/langchain4j/issues/1132

@Subh231004
Copy link
Author

Subh231004 commented Jun 22, 2024 via email

@ayewo
Copy link

ayewo commented Jun 22, 2024

I'm a beginner to the open source world and I'm trying hard to get to the problem statements I've had a short conversation with the maintainer of OpenAdapt and I have his permission to work on it I am just following whatever he is suggesting me

@Subh231004 I totally get that. I was merely commenting so Richard could calibrate his expectations accordingly.

@abrichr
Copy link
Member

abrichr commented Jun 22, 2024

Thank you @ayewo .

@Subh231004 thank you for your efforts. I appreciate your desire to contribute and I encourage you to continue. However this code is unfortunately not reviewable in its current state.

I suggest you try a simpler problem before attempting this one.

@abrichr abrichr closed this Jun 22, 2024
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