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

Update CL metadata in dm-reader #2894

Merged
merged 10 commits into from Mar 15, 2022

Conversation

jlaehne
Copy link
Contributor

@jlaehne jlaehne commented Feb 24, 2022

After fixing the metadata structure in LumiSpy, the mapping in the DM reader has to be updated. Implements the structure proposed in LumiSpy/lumispy#109 (and should be merged only after tis PR).

Furthermore, an SEM as microscope is now properly detected and the metadata set accordingly.

Follow up on #2590, where this mapping was first introduced. AS #2590 is still unreleased, this redesign should be included in the v1.7 release.

Progress of the PR

  • handle SEM as possible microscope instead of TEM,
  • update metadata according to Metadata structure LumiSpy/lumispy#109,
  • add a 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)
  • update tests,
  • ready for review.

@codecov
Copy link

codecov bot commented Feb 24, 2022

Codecov Report

Merging #2894 (1503b9f) into RELEASE_next_minor (f3aaab5) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                  Coverage Diff                   @@
##           RELEASE_next_minor    #2894      +/-   ##
======================================================
+ Coverage               79.10%   79.18%   +0.08%     
======================================================
  Files                     206      206              
  Lines                   31907    31872      -35     
  Branches                 7181     7187       +6     
======================================================
- Hits                    25239    25237       -2     
+ Misses                   4909     4876      -33     
  Partials                 1759     1759              
Impacted Files Coverage Δ
hyperspy/io_plugins/digital_micrograph.py 76.30% <100.00%> (+7.38%) ⬆️
hyperspy/misc/eels/eelsdb.py 60.86% <0.00%> (-4.35%) ⬇️
hyperspy/utils/peakfinders2D.py 91.06% <0.00%> (-1.17%) ⬇️
hyperspy/learn/ornmf.py 87.23% <0.00%> (-1.13%) ⬇️
hyperspy/learn/rpca.py 87.44% <0.00%> (-0.86%) ⬇️
hyperspy/_signals/signal2d.py 80.40% <0.00%> (-0.68%) ⬇️
hyperspy/_signals/lazy.py 91.85% <0.00%> (-0.66%) ⬇️
hyperspy/drawing/image.py 73.83% <0.00%> (-0.24%) ⬇️
hyperspy/drawing/utils.py 74.32% <0.00%> (-0.19%) ⬇️
hyperspy/model.py 80.47% <0.00%> (-0.12%) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3aaab5...1503b9f. Read the comment docs.

@jlaehne jlaehne added this to the v1.7 milestone Feb 24, 2022
@jlaehne jlaehne self-assigned this Feb 24, 2022
Copy link
Member

@ericpre ericpre left a comment

Choose a reason for hiding this comment

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

Looks good, I have left some suggestions for improvement.

hyperspy/io_plugins/digital_micrograph.py Outdated Show resolved Hide resolved
hyperspy/io_plugins/digital_micrograph.py Outdated Show resolved Hide resolved
@jlaehne
Copy link
Contributor Author

jlaehne commented Mar 14, 2022

Thanks @ericpre. Two very good points that I took up.

Looking if there are easy ways to increase coverage, I realized that the functions find_next_tag and find_next_data_tag are not called anywhere and seem to be remnants from some early implementation. Any reason against removing them?

Also, having a file with complex data would do a big job for the coverage.

@ericpre ericpre merged commit 76038e3 into hyperspy:RELEASE_next_minor Mar 15, 2022
@ericpre
Copy link
Member

ericpre commented Mar 15, 2022

Looking if there are easy ways to increase coverage, I realized that the functions find_next_tag and find_next_data_tag are not called anywhere and seem to be remnants from some early implementation. Any reason against removing them?

I guess, this have been used for debugging purposes but this is not documented, so I agree that this is fine to tidy it up.

@jlaehne jlaehne deleted the dm_cl-metadata-update branch March 16, 2022 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants