Skip to content

Implement proxy and multithreading#54

Merged
KyleQianliMa merged 15 commits intonextfrom
ewm13859implement_proxy_and_multithreading
Feb 2, 2026
Merged

Implement proxy and multithreading#54
KyleQianliMa merged 15 commits intonextfrom
ewm13859implement_proxy_and_multithreading

Conversation

@walshmm
Copy link
Contributor

@walshmm walshmm commented Jan 13, 2026

Short description of the changes:

This pr implements some necessary threading changes and reorgs the project structure a little, if I need to revert or move anything let me know, but I needed to do so to correctly test and setup threading.

I have Removed the prototype code.

Long description of the changes:

  • reorganized project structure and removed prototype
  • implemented threading Signals independant of pyqt
  • reorganized main and application startup
  • added singleton decorator
  • implemented thread worker with various signals to indicate start, finish, return
  • proxies no longer return any values, that defeats the point of threading

Check list for the pull request

  • I have read the [CONTRIBUTING]
  • I have read the [CODE_OF_CONDUCT]
  • I have added tests for my changes
  • I have updated the documentation accordingly

Check list for the reviewer

  • I have read the [CONTRIBUTING]
  • I have verified the proposed changes
  • best software practices
    • all internal functions have an underbar, as is python standard
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date
  • code comments added when explaining intent

Manual test for the reviewer

References

* reorganized project structure and removed prototype
* implemented threading Signals independant of pyqt
* reorganized __main__ and application startup
* added singleton decorator
* implemented thread worker with various signals to indicate start, finish, return
* proxies no longer return any values, that defeats the point of threading
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2026

Codecov Report

❌ Patch coverage is 91.55844% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.51%. Comparing base (bdd1888) to head (f683253).

Files with missing lines Patch % Lines
src/tavi/meta/decorators/singleton.py 78.78% 7 Missing ⚠️
src/tavi/meta/multithreading/signal.py 88.46% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             next      #54       +/-   ##
===========================================
+ Coverage   62.85%   82.51%   +19.65%     
===========================================
  Files           3        6        +3     
  Lines          70      223      +153     
===========================================
+ Hits           44      184      +140     
- Misses         26       39       +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@walshmm walshmm force-pushed the ewm13859implement_proxy_and_multithreading branch from 46efbd3 to 42e6b7f Compare January 14, 2026 19:04
@walshmm walshmm force-pushed the ewm13859implement_proxy_and_multithreading branch from 7fd9a57 to 5a2a6bb Compare January 14, 2026 19:29
@walshmm walshmm force-pushed the ewm13859implement_proxy_and_multithreading branch from 016b134 to b489aaa Compare January 14, 2026 20:39
@hetrickjm
Copy link

I would ask that we separate out prototype code into the prototype folder or eleminate it all together if we have a fork. Also, I would prefer to see the classes have just the current implementation to this point. There is a good bit of "prototype" code and it is hard to distinguish what should be there and what should not. For instance, the threading to load files in the TaviProjectModel should not be there along with some of the attributes. It would make the review of what was actually added or changed for implementing threading much easier

@walshmm walshmm marked this pull request as ready for review January 23, 2026 19:00
Copy link
Member

@KyleQianliMa KyleQianliMa left a comment

Choose a reason for hiding this comment

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

I had two more comments. Let's keep the scope of the PR contained to only multithreading and proxy that should only involve the stuff in meta folder and model_response.

I'll see if I can use the singleton and proxy with the load folder gui. Then I'm happy to approve. Please also fix pre-commits. I know it's tedious but Andrei decided to enforce strict rules for tavi so let's respect that.

@@ -0,0 +1,56 @@
"""Main Qt entry point."""
Copy link
Member

Choose a reason for hiding this comment

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

delete this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a strong reason to remove this? Its just standard boilerplate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can separate it out into another pr, but I still think this stuff needs to get added.

Copy link
Member

Choose a reason for hiding this comment

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

Just to keep this pr contained. Currently there are 66 files involved for multithreading and proxy implementation. Best to do it in another pr.

@@ -1,37 +1,40 @@
"""Entry point."""
Copy link
Member

Choose a reason for hiding this comment

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

revert to __ main __.py in next branch. Restore "configuration.py", "configuration_template.ini", "version.py". Let's keep the scope contained only for multi-threading and proxy. So i'm only looking at "meta" folder and "model_response.py".

Copy link
Contributor Author

@walshmm walshmm Jan 28, 2026

Choose a reason for hiding this comment

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

of those I think I only altered version.py which really should be in the gitignore

Copy link
Contributor Author

@walshmm walshmm Jan 28, 2026

Choose a reason for hiding this comment

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

err, actually Im not sure why we event have a version.py? I know shiver does it that way, but otherwise I think we put it in the first __init__.py of the package, like in the template repo

Copy link
Member

Choose a reason for hiding this comment

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

yes, my bad. I got confused looking between branches and version.py.

Copy link
Member

Choose a reason for hiding this comment

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

In bing's original tavi I think there is a line he imported the version file. I would also do it in the next pr

@walshmm
Copy link
Contributor Author

walshmm commented Jan 28, 2026

please also fix pre-commits. I know it's tedious but Andrei decided to enforce strict rules for tavi so let's respect that.

sorry missed those two, they didnt show up on my local for some reason. (I did setup the percommit hook)

Copy link
Member

@KyleQianliMa KyleQianliMa left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@KyleQianliMa KyleQianliMa merged commit 502f0ab into next Feb 2, 2026
7 checks passed
@KyleQianliMa KyleQianliMa deleted the ewm13859implement_proxy_and_multithreading branch February 2, 2026 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants