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

Code should not change the root group's name #98

Closed
agcapps opened this issue Sep 17, 2019 · 3 comments
Closed

Code should not change the root group's name #98

agcapps opened this issue Sep 17, 2019 · 3 comments
Assignees
Labels
bug Something isn't working Sidre Issues related to Axom's 'sidre' component

Comments

@agcapps
Copy link
Member

agcapps commented Sep 17, 2019

Currently code can change the name of the root group. However, it should not change from the empty string. Noah has a reason for this ("the kludge"); we didn't fully discuss it for lack of time.

@agcapps agcapps added bug Something isn't working Sidre Issues related to Axom's 'sidre' component labels Sep 17, 2019
@nselliott
Copy link
Contributor

I don't have an absolute objection to changing the root group's name. It shouldn't be automatically changed by load(), but that is the same for any Group, and we're addressing that in #99.

There is Group::rename() in the API, which can be called to change the name of the root group, the same as any other group. If a user code wants to do this, I thing it should continue to be allowed. It then is the user code's responsibility to keep track of the fact that it has changed names.

The kludge relates to the current behavior of load(), which we intend to eradicate. Currently load changes the name of the calling Group to the name of the loaded Group, except if the loaded Group's name is the empty string. If the plan if #99 is fully implemented, this all becomes unnecessary.

There is documentation that claims the root group's name is "/". This should be corrected. The root group represents a concept analogous to the directory "/" in a file system, but it is not actually named "/". In fact the path delimiter character cannot be in the name of any Group. This is enforced implicitly when groups are constructed and explicitly when rename() is called.

@agcapps
Copy link
Member Author

agcapps commented Sep 20, 2019

As long as there are no technical problems, I agree that if a user code calls Group::rename() on a root group, the code should be able to change the root group name.

I'll work in issue #99 and come back to this one and fix the documentation.

@agcapps
Copy link
Member Author

agcapps commented Oct 8, 2019

PR #111 addressed this issue and is now merged.

@agcapps agcapps closed this as completed Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Sidre Issues related to Axom's 'sidre' component
Projects
None yet
Development

No branches or pull requests

2 participants