BCDA-APS / mdaviz

Data visualization for mda
https://bcda-aps.github.io/mdaviz/
Other
3 stars 0 forks source link

Implement multiple tabs in file tableview. #112

Closed rodolakis closed 4 months ago

rodolakis commented 5 months ago
prjemian commented 5 months ago

First thing that crashed it was trying to open a file outside of the standard test data. I presume this file has more than one dimension.

========= 7bmb1_0251.mda in /home/prjemian/Documents/test_mda_files/7bmb
Traceback (most recent call last):
  File "/home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_folder.py", line 408, in onFileSelected
    self.mda_file.addFileTab(index.row(), self.selectionField())
  File "/home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_file.py", line 292, in addFileTab
    self.setData(index)
  File "/home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_file.py", line 161, in setData
    file_metadata, file_data = readMDA(file_path)
    ^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: too many values to unpack (expected 2)

Using the mda.readMDA(fname), that function returns three items.

>>> contents = mda.readMDA(fname)
>>> len(contents)
3
>>> type(contents[0])
<class 'dict'>
>>> type(contents[1])
<class 'mda.scanDim'>
>>> type(contents[2])
<class 'mda.scanDim'>

For now, wrap this code with try..except that says mdaviz is not ready to open that file now.

prjemian commented 5 months ago

Screen from Windows: image

rodolakis commented 5 months ago

Using the mda.readMDA(fname), that function returns three items.

>>> contents = mda.readMDA(fname)
>>> len(contents)
3
>>> type(contents[0])
<class 'dict'>
>>> type(contents[1])
<class 'mda.scanDim'>
>>> type(contents[2])
<class 'mda.scanDim'>

For now, wrap this code with try..except that says mdaviz is not ready to open that file now.

info, * = mda.readMDA(fname)
print(f"{info.rank=}")
prjemian commented 5 months ago

Why are there two no_mda_folder entries? image

prjemian commented 5 months ago

Next and Back buttons do not act at this point: image

prjemian commented 5 months ago

Also, "Select first file" button causes application to crash:


doPlot called: action='replace', selection={'X': 1, 'Y': [2]}
Traceback (most recent call last):
  File "/home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_folder.py", line 556, in goToFirst
    self.selectAndShowIndex(firstIndex)
  File "/home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_folder.py", line 620, in selectAndShowIndex
    self.selectionModel().setCurrentIndex(
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'setCurrentIndex'
prjemian commented 5 months ago

Move the full path from a widget (that takes screen real estate) into a tooltip for its tab: image

prjemian commented 5 months ago

Path length pushes this panel to add a scroll bar for the graph: image

The path could be scrollable or some tooltip. Which widget, then? The ComboBox? Needs to be updated with selection.

prjemian commented 5 months ago

I won't comment on the .mda or .ui files directly. Review of the .ui files comes by review of the GUI itself.

rodolakis commented 4 months ago

Using the mda.readMDA(fname), that function returns three items.

>>> contents = mda.readMDA(fname)
>>> len(contents)
3
>>> type(contents[0])
<class 'dict'>
>>> type(contents[1])
<class 'mda.scanDim'>
>>> type(contents[2])
<class 'mda.scanDim'>

For now, wrap this code with try..except that says mdaviz is not ready to open that file now.

info, * = mda.readMDA(fname)
print(f"{info.rank=}")

Instead of a try... except... I did this:

        file_metadata, file_data_dim1, *_ = readMDA(file_path)
        if file_metadata["rank"] > 1:
            self.setStatus("WARNING: Multidimensional data not supported - ignoring ranks > 1.")
prjemian commented 4 months ago

Today's PR merges have created a couple merge conflicts to resolve.

prjemian commented 4 months ago
prjemian commented 4 months ago

I'll fix on my end and push back to this branch. I'll ignore your PR #122.

prjemian commented 4 months ago

Using a VScode session on my local Linux workstation, here's what I have done:

On the command line:

  1. git checkout main
  2. git pull
  3. git checkout 90-keep-relevant-file-tabs-open
  4. git pull

In the VSCode editor

  1. merge the main branch into the 90-keep-relevant-file-tabs-open
  2. delete the one file
  3. make the changes to the other file using the Merge Editor
  4. Commit the changes
  5. Sync (push) the commit back to GitHub
prjemian commented 4 months ago

About PR #122, seems that was handled automatically by GitHub. It's closed now as the merge completed.

prjemian commented 4 months ago

https://github.com/BCDA-APS/mdaviz/blob/79d64d6900ef5ad1f3eba5aeaec03fa0fd038087/mdaviz/utils.py#L24

Can the kilobyte entry be changed to "kB"? Lower case k is the standard.

rodolakis commented 4 months ago

Why are there two no_mda_folder entries? image

Done

rodolakis commented 4 months ago

Move the full path from a widget (that takes screen real estate) into a tooltip for its tab: image

I added the tooltip but at the moment I leaning towards keeping the filepath Qlabel above the tableview anyway. I am using it in the code to identify the path to index / index to path relationships. Removing it would involve some refactoring, and I think I personally prefer to have it explicitly displayed anyway. Will ask users, could be moved to 1.1 if needed.

rodolakis commented 4 months ago

Path length pushes this panel to add a scroll bar for the graph: image

The path could be scrollable or some tooltip. Which widget, then? The ComboBox? Needs to be updated with selection.

Replaced qlabel with tooltip. The tooltip is both on the qcombo box (updated at selection) AND on the individual items within the combo box.

rodolakis commented 4 months ago

I think I fixed all the stuff we talked about this afternoon:

@rodolakis PR https://github.com/BCDA-APS/mdaviz/pull/112 "unhighligh" previous tab/detector row after changing curve s… 31be473 @rodolakis PR https://github.com/BCDA-APS/mdaviz/pull/112 correctly reset selection model after selecting a different f… 56603f6 @rodolakis PR https://github.com/BCDA-APS/mdaviz/pull/112 fixed issues with clear graph & clear tab when in auto-add 1d17151 @rodolakis PR https://github.com/BCDA-APS/mdaviz/pull/112 displaying list of all positioner PVs in graph x label

rodolakis commented 4 months ago

@prjemian ready for re-review

prjemian commented 4 months ago

X axis label? image

prjemian commented 4 months ago

Console output could be suppressed in next version:

========= mda_0007.mda in /home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/data/test_folder1

----- Selection before clean up: {'X': 1, 'Y': [2]}
----- Selection After clean up: {'X': 1, 'Y': [2]}

doPlot called: action='add', selection={'X': 1, 'Y': [2]}

========= mda_0008.mda in /home/prjemian/Documents/projects/BCDA-APS/mdaviz/mdaviz/data/test_folder1

----- Selection before clean up: {'X': 1, 'Y': [2]}
----- Selection After clean up: {'X': 1, 'Y': [2]}

doPlot called: action='add', selection={'X': 1, 'Y': [2]}
prjemian commented 4 months ago

I'm puzzled about the difference between buttons "Clear Tabs" and "Clear Graph". At first, I clicked "Clear Tabs" but the graph did not clear. Expected the graph to clear.

I expect the other button "Clear Graph" should not close tabs, just clear the graph. But it also clears the tabs.

prjemian commented 4 months ago

Testing at APS workstation, a BUG when selecting the folder to open:

(mdaviz) jemian@otz ~/.../BCDA-APS/mdaviz $ Settings are saved in: /home/beams/JEMIAN/.config/BCDA-APS/mdaviz.ini
Application started ...
Please select a folder...
current_depth=4
current_depth=4
current_depth=4
Traceback (most recent call last):
  File "/home/beams1/JEMIAN/Documents/projects/BCDA-APS/mdaviz/mdaviz/mainwindow.py", line 250, in setSubFolderPath
    self.mvc_folder = MDA_MVC(self)
                      ^^^^^^^^^^^^^
  File "/home/beams1/JEMIAN/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_folder.py", line 101, in __init__
    self.setup()
  File "/home/beams1/JEMIAN/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_folder.py", line 107, in setup
    from .mda_file import MDAFile
  File "/home/beams1/JEMIAN/Documents/projects/BCDA-APS/mdaviz/mdaviz/mda_file.py", line 26, in <module>
    from mda import readMDA
ModuleNotFoundError: No module named 'mda'
prjemian commented 4 months ago

This line: https://github.com/BCDA-APS/mdaviz/blob/07ecdcee9d06e451899c6201f57e0405f0038dfc/mdaviz/mda_file.py#L26

should be changed to

from .synApps_mdalib.mda import readMDA