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

Move Sidre collections to core #1397

Open
rhornung67 opened this issue Aug 15, 2024 · 4 comments · May be fixed by #1443
Open

Move Sidre collections to core #1397

rhornung67 opened this issue Aug 15, 2024 · 4 comments · May be fixed by #1443
Assignees
Labels
Core Issues related to Axom's 'core' component design Issues related to design and software engineering Reviewed Sidre Issues related to Axom's 'sidre' component

Comments

@rhornung67
Copy link
Member

Sidre's map and item collections can be used in other components:

Since those classes are not specific to Axom and have no other Axom dependencies, it makes sense to move them to the Axom core component.

@rhornung67 rhornung67 added Core Issues related to Axom's 'core' component Sidre Issues related to Axom's 'sidre' component design Issues related to design and software engineering labels Aug 15, 2024
@rhornung67
Copy link
Member Author

Potential issue with get name function, which is not generic

@nselliott nselliott self-assigned this Oct 8, 2024
@nselliott
Copy link
Contributor

The one Axom dependency that exists in these collection classes is slic, as there are a few assertions and one warning. Removing them would have no effect on the functionality, but would increase risk in the case that calls are made that would have triggered the assertions or warning. Is there a reasonable alternative to slic in this case?

@bmhan12
Copy link
Contributor

bmhan12 commented Oct 9, 2024

In core, at least for assertions, the current way is to replace SLIC_ASSERT with C assert() to not have the dependency between core and slic: https://github.com/search?q=repo%3ALLNL%2Faxom+%22assert%22+path%3Asrc%2Faxom%2Fcore%2F*&type=code

@nselliott
Copy link
Contributor

Thanks @bmhan12, I can definitely do that for the assertions. For the warning, I suppose I should write a std::cerr message like in axom::Array since I don't have access to the Logger.

@nselliott nselliott linked a pull request Oct 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issues related to Axom's 'core' component design Issues related to design and software engineering Reviewed Sidre Issues related to Axom's 'sidre' component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants