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

Key error loading Bruker .bcf file #2881

Closed
TMoon96 opened this issue Feb 15, 2022 · 9 comments · Fixed by #2900
Closed

Key error loading Bruker .bcf file #2881

TMoon96 opened this issue Feb 15, 2022 · 9 comments · Fixed by #2900
Milestone

Comments

@TMoon96
Copy link

TMoon96 commented Feb 15, 2022

Screen Shot 2022-02-15 at 9 31 34 AM
Screen Shot 2022-02-15 at 9 17 07 AM
I'm trying to load a EDS map .bcf file using the most basic method (define path name and using hs.load(path), and get the error "Key error: 'Line'." Setting load_original_metadata to false does not resolve this. This file was recorded the same day as two others, one of which gives the same error but the third loads just fine. The files also open just fine in Bruker's ESPIRIT software.

@hakonanes
Copy link
Contributor

Hi @TMoon96,

I'm not very familiar with the .bcf IO plugin, but it seems to me like it is too strict, as this try/except block should also except your KeyError and move on:

def _set_elements(self, root):
"""wrap objectified xml part with selection of elements to
self.elements list
"""
try:
elements = root.find(
"./ClassInstance[@Type='TRTContainerClass']"
"/ChildClassInstances"
"/ClassInstance[@Type='TRTElementInformationList']"
"/ClassInstance[@Type='TRTSpectrumRegionList']"
"/ChildClassInstances")
for j in elements.findall(
"./ClassInstance[@Type='TRTSpectrumRegion']"):
tmp_d = dictionarize(j)
self.elements[tmp_d['XmlClassName']] = {'line': tmp_d['Line'],
'energy': tmp_d['Energy']}
except AttributeError:
_logger.info('no element selection present in the spectra..')

If you change the line to except (AttributeError, KeyError), at least your file reading won't fail on that part. Could you try that locally? If it works as expected, I suggest making a PR with that change, if maintainers agree.

@TMoon96
Copy link
Author

TMoon96 commented Feb 15, 2022

Thanks, that worked as you described. I'm wondering if I just didn't save the file with elements identified, but since the code should work in either case the change should probably still get made.

@TMoon96 TMoon96 closed this as completed Feb 15, 2022
@hakonanes
Copy link
Contributor

Thanks, that worked as you described.

Great!

I'm wondering if I just didn't save the file with elements identified, but since the code should work in either case the change should probably still get made.

I agree, you should be able to read either way. Do you want to make a PR?

@ericpre
Copy link
Member

ericpre commented Feb 16, 2022

It would be good to make a PR to fix it, if bruker software can open the file, it would be fair to expect hyperspy to be able to open it, even if some metadata are missing, even if we don't know why this specific file doesn't have this metadata. There is a tradeoff between letting the code fails to get user to report issue and convenience. In this case, maybe, a warning could be raised when loading the file?

@hakonanes
Copy link
Contributor

Raise the log entry to a warning, or in addition?

@ericpre
Copy link
Member

ericpre commented Feb 16, 2022

Catch the error to allow loading the file and raise a warning (using the logger?) to inform the user that some information may be missing.

@hakonanes
Copy link
Contributor

Currently, the try/except clause catches an attribute error, presumably raised by xml.etree.ElementTree.find() when some tree entry string isn't found, makes a log entry, and moves on.

except AttributeError:
_logger.info('no element selection present in the spectra..')

I think the log message describes the present error nicely, and suggest to just catch the present KeyError as well, and raise the log entry to a warning, because the warning will be added to the log anyway...?

@ericpre
Copy link
Member

ericpre commented Feb 16, 2022

Yes, that may be already good enough! If @TMoon96 can understand why it raises a different error using its file than the one that is currently expected, it would be good but this is not necessary. In this case, it would be useful to look at what are the keys of tmp_d?

@sem-geologist
Copy link
Contributor

sem-geologist commented Mar 24, 2022

@TMoon96, could You send the erroring bcf file? I guess that You had selected some elements. In some versions/implementations/adaptations of Esprit, some of XML nodes are not populated. Or at least tell us what elements had You selected. In some cases it could be that element has not much lines to chose from, and Esprit could try omit that information. the PR #2900 tries to skip past the error instead of handling it correctly, as a consequence You will get no list of elements selected in Hyperspy metadata. Proper fix, should still keep the parsed element (abbreviation), catch the missing Line and fill it automatically (I think the safest approach is default to KA line). I guess it can be so for low atomic number elements, but It would be really good to see the file which made this error.

@ericpre ericpre added this to the v1.7 milestone Mar 30, 2022
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 a pull request may close this issue.

4 participants