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

Direct loading data from databroker. ROI settings tab. #191

Merged
merged 86 commits into from
Oct 17, 2019

Conversation

dmgav
Copy link
Contributor

@dmgav dmgav commented Oct 10, 2019

Implemented and fixed features include:

-- direct loading data from databroker by scan ID is working. Remotely tested on SRX, HXN and TES.

-- improved consistency of behavior of make_hdf and pyxrf_batch functions. Both functions will raise exception if error occurred during the processing if called for a single scan/file or go to processing the next scan/file if the list of files or range of scan IDs is provided. make_hdf will also may detect 'incomplete' scans that have no stop document.

-- fixed and improved functionality of ROI setting tab, which allows to compute ROIs. All features are operational and stable. Added an option to subtract background from raw data before computing ROIs.

-- fixed minor stability issues, including program crash when removing all selected lines using Delete not selected items or Delete Rel Int buttons in Fit tab.

-- working directory is now set at start-up of PyXRF to current working directory.

-- calculated ROIs are saved in the HDF5 files. Separate set of ROIs are saved for each dataset (such as sum, det1, det2 etc.

-- all processing results contained in HDF5 file are always loaded. For example, if fitting or ROI results for det2 are saved in an HDF5 file, they will be loaded and available for viewing even if raw data from individual detector channels is not loaded (only sum is loaded).

-- buttons < and > are added to Fit and Spectrum View tab to switch between element emission lines (possible fix for issue with MacOS version of PyXRF, in which ComboBox doesn't seem to keep focus and swiching using Up and Down arrows does not work reliably).

Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I'd like to mention, that it's not possible to give a comprehensive review on such a lengthy PR with 60+ commits, more than a thousand line changes in 14 files. Each bullet point in the description deserves its own separate PR.

Anyway, here is my review.

pyxrf/gui.py Show resolved Hide resolved
pyxrf/model/command_tools.py Show resolved Hide resolved
pyxrf/model/command_tools.py Show resolved Hide resolved
pyxrf/model/fileio.py Outdated Show resolved Hide resolved
pyxrf/model/fileio.py Outdated Show resolved Hide resolved
pyxrf/model/load_data_from_db.py Show resolved Hide resolved
pyxrf/model/load_data_from_db.py Outdated Show resolved Hide resolved
pyxrf/model/setting.py Outdated Show resolved Hide resolved
pyxrf/model/setting.py Outdated Show resolved Hide resolved
pyxrf/model/setting.py Outdated Show resolved Hide resolved
dmgav and others added 17 commits October 17, 2019 09:15
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
Co-Authored-By: Maksim Rakitin <mrakitin@users.noreply.github.com>
@@ -181,18 +313,10 @@ def update_roi(self, element_list, std_ratio=4):
self.roi_dict.update({v: roi})

# remove old items not included in element_list
for k in six.iterkeys(self.roi_dict):
for k in self.roi_dict.copy().keys():
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one of many things in the original code that did not work as expected.

pyxrf/model/setting.py Outdated Show resolved Hide resolved
Copy link
Member

@mrakitin mrakitin left a comment

Choose a reason for hiding this comment

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

Thanks for addressing all of the comments from my previous review. Here are a few more.

Non-existing scans in the range or scans causing errors during conversion will be skipped.

Keyword parameters
------------------
Copy link
Member

Choose a reason for hiding this comment

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

In particular, I meant a separate Keyword parameters block. I think everything can be under the Parameters above.

num_end_lines_excluded=None):
"""
Transfer multiple h5 files.
Load data from database and save it in HDF5 files.

Parameters
---------
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
---------
----------

break
except Exception:
# Wait for 10 minutes and retry
time.sleep(600)
Copy link
Member

Choose a reason for hiding this comment

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

Can it be a configurable parameter? Otherwise a new version of the code will be needed each time a new sleep time is asked.

@mrakitin mrakitin merged commit 56b10dd into NSLS-II:master Oct 17, 2019
@dmgav dmgav deleted the databroker_load branch November 5, 2019 18:25
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