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

optimize get streams for BrukerTiffSinglePlaneImagingExtractor #344

Merged
merged 8 commits into from
Jun 12, 2024

Conversation

h-mayorquin
Copy link
Collaborator

This uses lazy xml reading instead of parsing the whole xml file. On my data this implementation is 60 times faster.

@h-mayorquin h-mayorquin self-assigned this Jun 11, 2024
Copy link
Contributor

@weiglszonja weiglszonja 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, thanks @h-mayorquin. Should we add this in the changelog?

@h-mayorquin
Copy link
Collaborator Author

Looks good, thanks @h-mayorquin. Should we add this in the changelog?

Done.

Comment on lines +346 to +356
channel_names = set()
for event, elem in ElementTree.iterparse(xml_file_path, events=("start",)):
if elem.tag == "Frame":
# Get all the sub-elements in this Frame element
for subelem in elem:
if subelem.tag == "File":
channel_names.add(subelem.attrib["channelName"])

break

return channel_names
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to switch to lxml as in #346 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea. Do you mind if I switch it in #346 just to avoid merging conficts? All of these PR are linked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, sounds good. thanks!

@h-mayorquin h-mayorquin merged commit a3d0867 into main Jun 12, 2024
29 checks passed
@h-mayorquin h-mayorquin deleted the improve_bruker_get_streams branch June 12, 2024 14:55
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