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

silx.io.nxdata: Updated get_default to be more permissive and follow @default recursively #3662

Merged
merged 3 commits into from
Sep 8, 2022

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Sep 7, 2022

This PR changes behavior of silx.io.nxdata.get_default.
The search of @default NXdata now recursively follows @default.
It also stop checking NX_class along the way (i.e., NXroot/NXentry/NXdata) and only checks for NX_class="NXdata" to find the NXdata group.

I'm wondering if the silx.io.nxdata functions: is_group_with_default_NXdata, is_NXentry_with_default_NXdata, is_NXroot_with_default_NXdata that were used by get_default are used anywhere (they're no longer used in silx), I'm tempted to deprecate them...

It still needs a bit of testing.

closes #3605

@t20100 t20100 added this to the 1.1.0 milestone Sep 7, 2022
@t20100 t20100 requested a review from woutdenolf September 7, 2022 15:14
@t20100 t20100 marked this pull request as ready for review September 8, 2022 10:01
@t20100
Copy link
Member Author

t20100 commented Sep 8, 2022

I added some tests and made it resilient to @default loops since absolute path are supported in @default

Copy link
Contributor

@woutdenolf woutdenolf left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@woutdenolf
Copy link
Contributor

woutdenolf commented Sep 8, 2022

TestDataFileDialogInteraction.testClickOnBackToParentTool fails on mac. Is it related to the changes?

@t20100
Copy link
Member Author

t20100 commented Sep 8, 2022

No, there's quite a lot of random issue with tests relying on timings at the moment like interaction in widgets or a file to be created/deleted. I didn't looked at this yet.

@t20100 t20100 merged commit 7ea288b into silx-kit:master Sep 8, 2022
@t20100 t20100 deleted the permissive-get_default branch September 8, 2022 14:30
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.

Nexus: mygroup@default must be a name in mygroup
2 participants