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

Add support for Jeol epma data format (Issue #115) #118

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nem1234
Copy link
Contributor

@nem1234 nem1234 commented May 16, 2023

Draft

This is not suitable for merging into main stream

more sample data is needed

Description of the change

  • Minimum support for JEOL EPMA map
  • Not enough metadata, filesize info
  • Image size is fixed. (256x256)

Progress of the PR

  • Change implemented (can be split into several points),
  • update docstring (if appropriate),
  • update user guide (if appropriate),
  • add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
  • Check formatting changelog entry in the readthedocs doc build of this PR (link in github checks)
  • add tests,
  • ready for review.

Minimal example of the bug fix or the new feature

from rsciio.jeol import file_reader
file_reader("your_msa_file.map")
# Your new feature...

This is just a minimum EPMA map support for discussion.
I don't know if new signal_type should be defined or not.

Add test for epma image
Replace tmpdir with pytest's tmp_path
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (e62e1d6) 85.26% compared to head (7d03ae9) 85.27%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #118      +/-   ##
==========================================
+ Coverage   85.26%   85.27%   +0.01%     
==========================================
  Files          73       73              
  Lines        9030     9051      +21     
  Branches     2045     2048       +3     
==========================================
+ Hits         7699     7718      +19     
- Misses        871      872       +1     
- Partials      460      461       +1     
Impacted Files Coverage Δ
rsciio/jeol/_api.py 99.18% <100.00%> (+0.03%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sem-geologist
Copy link
Contributor

sem-geologist commented May 16, 2023

@nem1234 , what do you mean signal type EPMA? "EPMA" is type of method and/ or specific analytical instrument (Electron probe micro-analysis or analyzer, depending from context). The signal should be either EDS or WDS... if it would have energy or wavelength dimension, but actually this is just "raw counts". If it would be background corrected (with subtracted background) it could be called "net counts" (but I highly doubt Jeol probe software is even able to do that). If it is from Jeol EPMA it is probably collected raw xray counts at fixed WDS position set at known X-ray line energy and scaled down to fit 8 bits. Taking into consideration that it even should not be called a "raw counts" - it is just simple image with no exposed relationship to counts.

However, is Rosettasciio the right place for static single 8 bit, fixed resolution maps? If this is implemented, then maybe I should add subset of reader for Cameca EPMA mapping files and WDS spectral scans too (which compared to this poor file is overwhelmingly metadata-wise feature complete)? I would doubt about that, but after seeing usage cases with PCA and other methods on EDS hypermaps I start to wonder if similar methods would not be applicable on stacked WDS raw (or better, net count intensity) maps. Or ultimately stacked k-ratio maps...

To fool-proof this implementation, I would ask the author to produce more such files with different resolutions and different maximum count rates - it could reveal if there is indeed no additional flags in header (it does not sound right that there is only fixed single 256x256 pixel ratio), maybe there are enumerates for resolution and bit depth, where lowest resolution of 256x256 happens to be binary 00, and 8 bit int also is a binary 00 in corresponding enum. At this point in my opinion there is a bit too much unknowns with this single file for implementation. It is not kind of bug fix, it is a whole new format being introduced.

@nem1234
Copy link
Contributor Author

nem1234 commented May 16, 2023

Thank you.
I recognized that this imprimentation is very poor, and does not cover whole JEOL EPMA files.
The file format seems different to current JEOL's TEM EDS format,
This file seems a "mapping", not a fullly colleced raw data set.

In my recognization, PR should be small, step-by-step commit to main tree.
In this stage, I only implement on Issue #115.
If I should not implement such limited purpose code, I'll retract this PR.

@nem1234 nem1234 closed this May 16, 2023
@CSSFrancis
Copy link
Member

@nem1234 you can always open a PR and set the PR as a draft! That helps to signal that you are still working on some things or that you might need help.

Getting feedback is a part of the process and it is nice to know what people are doing so things aren't duplicated! My general rule of thumb is try and get something working to the point where I can at least provide an example of an implementation and then submit a PR.

If you want you can reopen this PR and mark it as a draft then maybe we can think about generalization of the file format if we can find more methods.

@sem-geologist I'd say that more of these processed datasets do have a place in rosettasciio. If the idea is to reduce the dependency on 3rd party applications I think that's pretty well in line but we could open an issue or discussion.

@sem-geologist
Copy link
Contributor

sem-geologist commented May 16, 2023

wait, is it not possible to get some more data from the author of this file? I think this could be good to have. As I said, during EMAS workshop I had seen really relevant presentations utilizing HyperSpy behind. Multi-WDS instruments (basically one of optional characteristics of EPMA instrument) can produce more concentrated view on X-ray generated in the sample. As I said I see potential of all these advanced methods built inside hyperspy, where instead of EDS spectral image, stacked raw or net count WDS cube could be used. Blind source separation, PCA and other methods should work same and maybe even better than on EDS, as this would be much less noisy measurements and much more sensitive to low concentrations. While HyperSpy lacks on X-ray quantification, I got convinced that not always there is a need or even is impossible to use standard based quantification. So please don't close this PR, it would motivate me to bring in Cameca formats here too.

@nem1234 nem1234 reopened this May 16, 2023
@nem1234 nem1234 marked this pull request as draft May 16, 2023 11:12
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