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

Renaming and moving #151

Merged
merged 27 commits into from
Mar 1, 2019
Merged

Renaming and moving #151

merged 27 commits into from
Mar 1, 2019

Conversation

JeanBilheux
Copy link
Contributor

All the work described in #141 has been done.

To test this pull request, simply check the new names of the tabs, play around and make sure is broken.

@marshallmcdonnell marshallmcdonnell self-requested a review February 25, 2019 21:09
@marshallmcdonnell marshallmcdonnell self-assigned this Feb 25, 2019
@marshallmcdonnell
Copy link
Member

marshallmcdonnell commented Feb 25, 2019

So, I came across this error when trying to load files using the original IDL ui for post-processing:

Traceback (most recent call last):
  File "/SNS/users/ntm/direnv/python2/addie/build/lib/addie/processing/idl/load_table_intermediate_step_interface.py", line 25, in ok_clicked
    self.parent.move_to_folder_step2()
  File "build/scripts-2.7/addie", line 2367, in move_to_folder_step2
    self.populate_table_clicked()
  File "build/scripts-2.7/addie", line 2383, in populate_table_clicked
    self.name_search_clicked()
  File "build/scripts-2.7/addie", line 2420, in name_search_clicked
    o_table.name_search()
AttributeError: TableHandler instance has no attribute 'name_search'

It looks like we import two different TableHandler classes here and here in the main scripts/addie:
Command:

grep "import TableHandler" scripts/addie

Out:

from addie.processing.idl.table_handler import TableHandler
from addie.processing.mantid.master_table.selection_handler import TableHandler

Looking, one has the name_search defined and the other does not. I assume we want to be using the first TableHandler we import:

grep TableHandler addie/processing/idl/table_handler.py && grep name_search addie/processing/idl/table_handler.py

Out:

class TableHandler(object):
    def name_search(self):
        _string = str(self.parent.name_search.text()).lower()
grep TableHandler addie/processing/mantid/master_table/selection_handler.py && grep name_search addie/processing/mantid/master_table/selection_handler.py

Out:

class TableHandler(SelectionHandlerMaster):
        self.parent.ui.name_search_3.setText("")

Copy link
Member

@marshallmcdonnell marshallmcdonnell left a comment

Choose a reason for hiding this comment

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

The plotting for the Calculate G(r) Tab is working as excpected(with any issues already reported as issues prior to these changes) and currently testing the Rietveld tab.

Currently, crashes for IDL Post-Processing due to the noted TableHandler error. Cannot continue with manual testing for this tab. Once this is fixed, I'll continue to test it out and review.

@marshallmcdonnell marshallmcdonnell dismissed their stale review February 25, 2019 21:34

Wrong choice for reveiw

@marshallmcdonnell marshallmcdonnell self-requested a review February 25, 2019 21:35
Copy link
Member

@marshallmcdonnell marshallmcdonnell left a comment

Choose a reason for hiding this comment

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

The plotting for the Calculate G(r) Tab is working as excpected(with any issues already reported as issues prior to these changes) and currently testing the Rietveld tab.

Currently, crashes for IDL Post-Processing due to the noted TableHandler error. Cannot continue with manual testing for this tab. Once this is fixed, I'll continue to test it out and review.

@marshallmcdonnell
Copy link
Member

The Rietveld tab is working as expected (except for bugs reported in #80 and #160)
Fix for #147 is now included in this PR.

@marshallmcdonnell
Copy link
Member

Going to open separate tickets to address these issues. Namely, issue #162

@marshallmcdonnell marshallmcdonnell merged commit 962f122 into master Mar 1, 2019
@marshallmcdonnell marshallmcdonnell deleted the renaming_and_moving branch March 4, 2019 13:47
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.

2 participants