Skip to content

Commit

Permalink
Merge pull request #18 from jordantgh/minor_gui_bugfixes_Oct2023
Browse files Browse the repository at this point in the history
Bugfixes oct2023

To enable faithful tracking of checked states per article across
different pages, added a lot of new logic.

Basic idea is the article.checked property is a dataclass with 
distinct properties for each page. These are then updated and
referred to by the UI.

Additionally added conditional handling of column pruning
logic depending on whether the source of the call to prune
was from the parsed tables page or the pruned tables page.

These changes prevent a number of strange behaviours
that I was observing due to varying and unpredictable
asynchronicities between the UI and model.
  • Loading branch information
jordantgh authored Oct 7, 2023
2 parents e1f19a3 + 45fa8b4 commit 20e8510
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 159 deletions.
108 changes: 70 additions & 38 deletions app/controller/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
from PyQt5.QtWidgets import QMessageBox
from enum import Enum, auto


class Mode(Enum):
BROWSING = 0
SEARCHING = auto()
PROCESSING = auto()
PRUNING = auto()


class Controller:
def __init__(self, model, view):
self.state = Mode.BROWSING
Expand All @@ -20,7 +22,7 @@ def __init__(self, model, view):

def set_state(self, state):
self.state = state

@property
def view_elem(self):
return self.view.active_elements
Expand Down Expand Up @@ -84,30 +86,32 @@ def connect_sigs(self):

def on_article_discovered(self, article_json, progress):
article_data = self.model.create_article_data(article_json)
self.view.display_article(self.search_page, article_data, progress)
self.view.display_article(
self.search_page, 'search', article_data, progress)

def on_article_processed(self, article, ids_list, progress):
article_data = self.model.update_article(article, ids_list)
self.view.display_article(self.parsed_page, article_data, progress)
self.view.display_article(
self.parsed_page, 'parsed', article_data, progress)

def search_for_articles(self):
self.set_state(Mode.SEARCHING)
self.model.reset_for_searching()
self.view.tab_widget.setCurrentIndex(0)

if self.model.search_thread.isRunning():
QMessageBox.warning(
self.view,
"Search in Progress",
"A search is already in progress. "
"A search is already in progress. "
"Please wait or stop the current search.")
return

self.model.search_thread.should_stop = False
query = self.search_page.query_field.text()
if not query:
return

# TODO these are low level concerns that should be handled by the view
self.search_page.article_list.clear()
self.search_page.previews.hide()
Expand All @@ -116,24 +120,24 @@ def search_for_articles(self):
self.search_page.search_status.setText("Searching...")
self.search_page.stop_search_btn.show()
self.search_page.stop_search_btn.setEnabled(True)

self.model.search_thread.query = query
self.model.search_thread.start()

def stop_search(self, search_thread):
search_thread.quit()

# TODO these are low level concerns that should be handled by the view
self.search_page.search_status.setText("Stopping search...")
self.search_page.prog_bar.hide()
self.search_page.prog_bar.hide()
self.search_page.search_status.setText("Search stopped.")
self.search_page.stop_search_btn.hide()
self.search_page.stop_search_btn.setEnabled(False)

self.set_state(Mode.BROWSING)

def send_search_stop(self):
if self.model.search_thread.isRunning():
if self.model.search_thread.isRunning():
self.model.search_thread.stop()
self.model.search_thread.wait()

Expand All @@ -159,8 +163,9 @@ def handle_article_click(self, item):
self.view.update_article_display(
article,
'supp_files',
self.view.suppfilelistitem_factory)

self.view.suppfilelistitem_factory,
'search')

# TODO do we need to pass in the factory if the relevant class can be
# derived from list item type within view.update_article_display?
# ... could just do away with the factories altogether
Expand All @@ -178,7 +183,8 @@ def handle_processed_article_click(self, item):
self.view.update_article_display(
article,
'processed_tables',
self.view.processedtablelistitem_factory)
self.view.processedtablelistitem_factory,
'parsed')

for i in range(self.view_elem.supp_files_view.count()):
list_item = self.view_elem.supp_files_view.item(i)
Expand All @@ -189,18 +195,19 @@ def handle_pruned_article_click(self, item):
self.view_elem.previews.hide()
article_id = item.data(Qt.UserRole)
article = self.model.bibliography.get_article(article_id)

self.view.update_article_display(
article,
'pruned_article_tables',
self.view.processedtablelistitem_factory)
self.view.processedtablelistitem_factory,
'pruned')

for i in range(self.view_elem.supp_files_view.count()):
list_item = self.view_elem.supp_files_view.item(i)
widget = self.view_elem.supp_files_view.itemWidget(list_item)
widget.preview_requested.connect(self.preview_pruned_table)

def load_preview(self, data, table_id=None, callback=None):
def load_preview(self, data, table_id=None, callback=None, context=None):
use_checkable_header = self.view_elem \
.__class__.__name__ == 'ProcessedPageElements'

Expand All @@ -212,8 +219,13 @@ def load_preview(self, data, table_id=None, callback=None):
processed_table = self.model.processed_table_manager \
.get_processed_table(table_id) if table_id else None

checked_columns = processed_table \
.checked_columns if processed_table else None
checked_columns = None
if context == 'parsed':
checked_columns = processed_table \
.checked_columns if processed_table else None
elif context == 'pruned':
checked_columns = processed_table \
.pruned_columns if processed_table else None

self.view.display_multisheet_table(
data, use_checkable_header, table_id, callback, checked_columns)
Expand All @@ -223,29 +235,31 @@ def load_preview(self, data, table_id=None, callback=None):
def preview_supp_file(self, file_id):
file_data = self.model.file_manager.get_file(file_id)
self.view.start_load_animation()

if self.model.preview_thread.isRunning():
self.model.preview_thread.quit()
self.model.preview_thread.wait()

self.model.preview_thread.file_url = file_data.url
self.model.preview_thread.start()

def preview_processed_table(self, table_id):
table_data = {
"sheet": self.model.table_db_manager.get_processed_table_data(
table_id)}

self.view.start_load_animation()
self.load_preview(table_data, table_id, self.update_checked_columns)
self.load_preview(table_data, table_id,
self.update_checked_columns, 'parsed')

def preview_pruned_table(self, table_id):
table_data = {
"sheet": self.model.table_db_manager.get_post_pruning_table_data(
table_id)}

self.view.start_load_animation()
self.load_preview(table_data, table_id, self.update_checked_columns)
self.load_preview(table_data, table_id,
self.update_pruned_columns, 'pruned')

def update_checked_columns(self, table_id, checked_columns):
processed_table = self.model.processed_table_manager \
Expand All @@ -254,16 +268,30 @@ def update_checked_columns(self, table_id, checked_columns):
if processed_table:
processed_table.checked_columns = checked_columns

def update_pruned_columns(self, table_id, checked_columns):
processed_table = self.model.processed_table_manager \
.get_processed_table(table_id)

if processed_table:
processed_table.pruned_columns = checked_columns

def prune_tables_and_columns(self):
# get current page from view_elem (class name doesnt work!)
current_page = self.view.tab_widget.currentIndex()
if current_page == 1:
context = 'parsed'
elif current_page == 2:
context = 'pruned'

self.view.tab_widget.setCurrentIndex(2)
self.model.prune_tables_and_columns()
self.model.prune_tables_and_columns(context)

self.view.clear_article_list_and_files_view()

# display all selected articles in the pruned page
# TODO need to make it so articles w/ no tables dont appear here
for article in self.model.bibliography.get_selected_articles():
self.view.display_article(self.pruned_page, article, 0)

for article in self.model.bibliography.get_selected_articles(context):
self.view.display_article(self.pruned_page, 'pruned', article, 0)

def filter_tables(self):
query = self.view_elem.query_filter_field.text()
if not query:
Expand All @@ -279,15 +307,15 @@ def on_proceed(self):
"Do you want to stop the current search and proceed?",
QMessageBox.Yes | QMessageBox.No,
QMessageBox.No)

if reply == QMessageBox.Yes:
self.send_search_stop()
else:
return
return

self.model.reset_for_processing()
self.view.tab_widget.setCurrentIndex(1)

if self.model.processing_thread.isRunning():
QMessageBox.warning(
self.view,
Expand All @@ -297,13 +325,17 @@ def on_proceed(self):

self.set_state(Mode.PROCESSING)
self.model.processing_thread.should_stop = False

# TODO these are low level concerns that should be handled by the view
self.view_elem.article_list.clear()
self.view_elem.previews.hide()
self.view_elem.prog_bar.setValue(0)
self.view_elem.prog_bar.show()

selected_articles = self.model.bibliography.get_selected_articles()

for article in self.model.bibliography.articles.values():
article.cascade_checked_state('search')

selected_articles = self.model.bibliography.get_selected_articles(
'search')
self.model.processing_thread.selected_articles = selected_articles
self.model.processing_thread.start()
Loading

0 comments on commit 20e8510

Please sign in to comment.