Skip to content

Convert core/edge to plasma IDS #87

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.

Sign up for GitHub

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

Jump to bottom
Open
maarten-ic wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Convert core/edge to plasma IDS #87

maarten-ic wants to merge 5 commits into from

Conversation

@maarten-ic
Copy link
Collaborator

@maarten-ic maarten-ic commented Dec 12, 2025

Fixes #44

@maarten-ic
Add conversion logic for core/edge -> plasma IDS
0aeeb22
@maarten-ic
Add docstrings for convert to plasma functions
1df6e82
@maarten-ic
Update __init__.py and docs
667dd62 
Expose additional API as "public" in the main `imas` space and update the Public API page accordingly.
@maarten-ic maarten-ic marked this pull request as ready for review December 15, 2025 14:23
@olivhoenen
Copy link
Collaborator

olivhoenen commented Jan 12, 2026 •
edited
Loading

Working fine for me. The option for convert CLI tool (item 3 in #44) was removed or simply postponed until we get occurrence_type in DD from @imbeauf ?

@maarten-ic
Copy link
Collaborator Author

maarten-ic commented Jan 13, 2026

The option for convert CLI tool (item 3 in #44) was removed or simply postponed [...]

I might have just forgotten to implement that, but don't recall to be honest.
What is your preference @olivhoenen ? I could add item 3 to this PR, except for the occurrence_type, if you like?

@olivhoenen
Copy link
Collaborator

olivhoenen commented Jan 13, 2026

Yes please do add it directly there, it will help some users while testing it, and we can amend it later once the DD is updated with new identitiers

@maarten-ic maarten-ic requested a review from IrisvdWerf January 13, 2026 09:30
olivhoenen
olivhoenen reviewed Jan 14, 2026
View reviewed changes
imas/command/cli.py
)

# Convert to plasma_profiles/plasma_sources/plasma_transport IDS
if convert_to_plasma_ids and ids_name.startswith(("core", "edge")):
Copy link
Collaborator

@olivhoenen olivhoenen Jan 14, 2026

Choose a reason for hiding this comment

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

I guess this test shall be improved, currently it will get also the core_instant_changes IDS (still have to find a data-entry with such IDS).

Otherwise it looks good to me, but I'm waiting an answer to iterorganization/IMAS-Data-Dictionary#191 before approving.

Copy link
Collaborator Author

@maarten-ic maarten-ic Jan 14, 2026

Choose a reason for hiding this comment

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

Sharp, I didn't think of that indeed! I'll update the logic

@maarten-ic maarten-ic requested a review from olivhoenen January 14, 2026 08:33
@maarten-ic maarten-ic mentioned this pull request Jan 14, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prasad-sawantdesai prasad-sawantdesai Awaiting requested review from prasad-sawantdesai prasad-sawantdesai is a code owner

@olivhoenen olivhoenen Awaiting requested review from olivhoenen olivhoenen is a code owner

1 more reviewer

@IrisvdWerf IrisvdWerf IrisvdWerf approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Converting core (resp. edge) to plasma_profiles/transport/sources

3 participants

@maarten-ic @olivhoenen @IrisvdWerf