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

Added a Data Channel Table to Tiled Browser #1

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

jryanlohmolder
Copy link

Data Channels can now be selected in Tiled Browser. Added methods to connect "ADD", "REPLACE", and "REMOVE" buttons.

Joshua Lohmolder and others added 14 commits July 8, 2024 15:00
…r class in it. All the code containing the plot window has been commented out.
…The key value pair for the former was the TiledDataSource.SOURCE_TYPE and TiledDataSource.TiledDataSource. The later was TiledDataSource.SOURCE_TYPE and QTiledWidget.TiledBrowser
…s clicked the Tiled Browser will be opened in the tab.
…py seems necessary if we want a Tiled Tab which holds the Tiled Browser. There may be a module we can reference in Tiled repo in TiledFile.py or maybe instead of TiledFiled.py.
…thods to add, replace, and remove scans. The methods need to be completed.
Copy link

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

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

Great work on this already!

I've made a few suggestions and comments in the review. Next I'll try running the code in this branch.

Comment on lines 50 to 51
from PyMca5.PyMcaGui.pymca import ScanWindow
from PyMca5.PyMcaGui.pymca import McaCalibrationControlGUI
Copy link

@padraic-shafer padraic-shafer Jul 30, 2024

Choose a reason for hiding this comment

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

In the spirit of minimizing changes to the upstream source code.

Suggested change
from PyMca5.PyMcaGui.pymca import ScanWindow
from PyMca5.PyMcaGui.pymca import McaCalibrationControlGUI
from PyMca5.PyMcaGui.pymca import McaCalibrationControlGUI
from PyMca5.PyMcaGui.pymca.ScanWindow import ScanWindow

@@ -74,15 +74,16 @@
# _logger.setLevel(logging.DEBUG


class McaWindow(ScanWindow):
def __init__(self, parent=None, name="Mca Window", specfit=None, backend=None,
class McaWindow(ScanWindow.ScanWindow):

Choose a reason for hiding this comment

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

In coordination with the change above to the import statement...

Suggested change
class McaWindow(ScanWindow.ScanWindow):
class McaWindow(ScanWindow):

Comment on lines 11 to 12
sigTiledDataChannelTableSignal = qt.pyqtSignal(object)
def __init__(self, parent=None):

Choose a reason for hiding this comment

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

[Formatting for clarity]

Suggested change
sigTiledDataChannelTableSignal = qt.pyqtSignal(object)
def __init__(self, parent=None):
sigTiledDataChannelTableSignal = qt.pyqtSignal(object)
def __init__(self, parent=None):

Comment on lines 55 to 56
for i in range(n):
self._addLine(i, channelList[i])

Choose a reason for hiding this comment

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

Pythonic loop permits a more descriptive name for the list item.

Suggested change
for i in range(n):
self._addLine(i, channelList[i])
for (i, channelLabel) in enumerate(channelList):
self._addLine(i, channelLabel)

item.setFlags(qt.Qt.ItemIsEnabled)

# Checkboxes
for j in range(1, 4):

Choose a reason for hiding this comment

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

Rather than hard-code the number of columns here, perhaps this value can be extracted from a table property; something like this?

Suggested change
for j in range(1, 4):
for j in range(1, self.columnCount()):

# 'x' column
if col == 1:
if ddict["state"]:
if row not in self.xSelection:

Choose a reason for hiding this comment

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

We should think about replacing this list-based logic with set-based logic. That can wait until a future discussion though.

https://www.w3schools.com/python/python_sets.asp

Comment on lines +132 to +137
if i in self.xSelection:
if not widget.isChecked():
widget.setChecked(True)
else:
if widget.isChecked():
widget.setChecked(False)

Choose a reason for hiding this comment

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

Possible simplification... see whether you find this more or less clear.

Suggested change
if i in self.xSelection:
if not widget.isChecked():
widget.setChecked(True)
else:
if widget.isChecked():
widget.setChecked(False)
widget.setChecked(
i in self.xSelection
and not widget.isChecked()
)

Comment on lines 24 to 29
def __init__(self, nameInput):
self.nameList = nameInput
self.source_type = SOURCE_TYPE
self.__sourceNameList = self.__sourceNameList
self._sourceObjectList = []
self.refresh()

Choose a reason for hiding this comment

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

I think I see a few problems here:

  1. Need to verify that nameInput is a list.
  2. self.__sourceNameList is self-defined (cyclic assignment)
  3. self.NameList should be defined

Here is a suggestion, based on NexusDataSource.

Suggested change
def __init__(self, nameInput):
self.nameList = nameInput
self.source_type = SOURCE_TYPE
self.__sourceNameList = self.__sourceNameList
self._sourceObjectList = []
self.refresh()
def __init__(self, nameInput):
if isinstance(nameInput, list):
nameList = nameInput
else:
nameList = [nameInput]
self.sourceName = []
self.source_type = SOURCE_TYPE
self.__sourceNameList = self. self.sourceName
self._sourceObjectList = []
self.refresh()

sourcename=sys.argv[1]
key =sys.argv[2]
except Exception:
print("Usage: NexusDataSource <file> <key>")

Choose a reason for hiding this comment

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

Suggested change
print("Usage: NexusDataSource <file> <key>")
print("Usage: TiledDataSource <url> <resource-path>")

Comment on lines 34 to 95
def getSourceInfo(self):
"""
Returns a dictionary with the key "KeyList" (list of all available keys
in this source). Each element in "KeyList" has the form 'n1.n2' where
n1 is the source number and n2 entry number in file both starting at 1.
"""
return self.__getSourceInfo()

def __getSourceInfo(self):
SourceInfo={}
SourceInfo["SourceType"]=SOURCE_TYPE
SourceInfo["KeyList"]=[]
i = 0
for sourceObject in self._sourceObjectList:
i+=1
nEntries = len(sourceObject["/"].keys())
for n in range(nEntries):
SourceInfo["KeyList"].append("%d.%d" % (i,n+1))
SourceInfo["Size"]=len(SourceInfo["KeyList"])
return SourceInfo

def getKeyInfo(self, key):
if key in self.getSourceInfo()['KeyList']:
return self.__getKeyInfo(key)
else:
#should we raise a KeyError?
_logger.debug("Error key not in list ")
return {}

def __getKeyInfo(self,key):
try:
index, entry = key.split(".")
index = int(index)-1
entry = int(entry)-1
except Exception:
#should we rise an error?
_logger.debug("Error trying to interpret key = %s", key)
return {}

sourceObject = self._sourceObjectList[index]
info = {}
info["SourceType"] = SOURCE_TYPE
#doubts about if refer to the list or to the individual file
info["SourceName"] = self.sourceName[index]
info["Key"] = key
#specific info of interest
info['FileName'] = sourceObject.name
return info

def getDataObject(self):
pass

def isUpdated(self, sourceName, key):
#sourceName is redundant?
index, entry = key.split(".")
index = int(index)-1
lastmodified = os.path.getmtime(self.__sourceNameList[index])
if lastmodified != self.__lastKeyInfo[key]:
self.__lastKeyInfo[key] = lastmodified
return True
else:
return False

Choose a reason for hiding this comment

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

I realize that this is coming from mostly from the NexusDataSource example. It looks like this will need to get modified with the results coming out of the QTiledWidget if I'm following it correctly.

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