8000 Transmission reader: Fixes reading of detector ranges by domna · Pull Request #112 · FAIRmat-NFDI/pynxtools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Transmission reader: Fixes readin 8000 g of detector ranges #112

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

Merged
merged 2 commits into from
May 19, 2023

Conversation

domna
Copy link
Collaborator
@domna domna commented May 19, 2023

There was an error in the reading of asc files if the measurement was only performed in one detector range. In this case no separate slits, gain and times are noted in the file which was failing during reading of the file.

@coveralls
Copy link
coveralls commented May 19, 2023

Pull Request Test Coverage Report for Build 5024542208

  • 8 of 9 (88.89%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 53.708%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pynxtools/dataconverter/readers/transmission/reader.py 7 8 87.5%
Totals Coverage Status
Change from base Build 4926761525: -0.01%
Covered Lines: 5055
Relevant Lines: 9412

💛 - Coveralls

@domna domna requested a review from sherjeelshabih May 19, 2023 13:13
Comment on lines +167 to +169
detector_slits[1:],
detector_times[1:],
detector_gains[1:],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing this is the important change here? This looks fine how its being used below. If so, this is ready to merge. I'll approve this unless something else needs attention to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this and the other line I added a comment for. Sorry that I committed my auto-formatting here, too

"PMT", detector_slits[2], detector_times[2],
None, 2, [wavelength_ranges[0], wavelength_ranges[1]]
"PMT",
detector_slits[-1],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is also relevant to use -1 instead of 2 here. This line was actually failing if there was only one detector

@domna domna merged commit dc07cb7 into master May 19, 2023
@domna domna deleted the fix-transmission-det-ranges branch May 19, 2023 13:30
RubelMozumder pushed a commit that referenced this pull request Jun 28, 2023
* Fixes reading of detector ranges

* Make linting happy
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.

3 participants
0