Skip to content

✨ Refactor type hints - #67

Merged
olivhoenen merged 2 commits into
iterorganization:developfrom
munechika-koyo:fix/type-annotation
Sep 11, 2025
Merged

✨ Refactor type hints#67
olivhoenen merged 2 commits into
iterorganization:developfrom
munechika-koyo:fix/type-annotation

Conversation

@munechika-koyo

@munechika-koyo munechika-koyo commented Sep 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation

When I used xml_path for DBEntry, which I retrieved from imas_data_dictionary.idsinfo.IDSInfo.get_idsdef_path(), I noticed that there was an annotation error because it returned the instance of pathlib.Path.

Summary

Allow pathlib.Path instance for the xml_path argument and Update type hints to PEP 604 syntax for improved readability and consistency.

Key changes

  • Add from __future__ import annotation to introduce PEP 604 syntax even into the python v3.8 and v3.9.
  • Fllow PEP 585, like use list[str] not List[str]
  • Apply pathlib.Path class type annotation to xml_path.

@munechika-koyo

munechika-koyo commented Sep 10, 2025

Copy link
Copy Markdown
Contributor Author

Did the documentation test error originate from here?

@olivhoenen

olivhoenen commented Sep 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Did the documentation test error originate from here?

I don't think so, this red scary-looking "error" is a warning and normal behavior when you don't have imas-core available (expected as it is not yet open-source 😢 ).

I believe the issue is more linked to

/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages/imas/db_entry.py:docstring of imas.db_entry.DBEntry:1: WARNING: py:class reference target not found: Path
/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages/imas/db_entry.py:docstring of imas.db_entry.DBEntry:1: WARNING: py:class reference target not found: Path
/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages/imas/db_entry.py:docstring of imas.db_entry.DBEntry.__init__:1: WARNING: py:class reference target not found: Path
/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages/imas/db_entry.py:docstring of imas.db_entry.DBEntry.__init__:1: WARNING: py:class reference target not found: Path
/opt/hostedtoolcache/Python/3.12.11/x64/lib/python3.12/site-packages/imas/db_entry.py:docstring of imas.db_entry.DBEntry.__init__:1: WARNING: py:class reference target not found: Path

Cf. sphinx-doc/sphinx#13731

@munechika-koyo

munechika-koyo commented Sep 11, 2025

Copy link
Copy Markdown
Contributor Author

@olivhoenen Thank you for your comment!
I switched to importing pathlib directly instead of Path, which seemed to pass the CI correctly.

olivhoenen
olivhoenen approved these changes Sep 11, 2025
View reviewed changes

@olivhoenen olivhoenen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@olivhoenen
olivhoenen merged commit 504b997 into iterorganization:develop Sep 11, 2025
12 checks passed
@munechika-koyo
munechika-koyo deleted the fix/type-annotation branch September 11, 2025 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olivhoenen olivhoenen olivhoenen approved these changes
@maarten-ic maarten-ic Awaiting requested review from maarten-ic maarten-ic is a code owner
@prasad-sawantdesai prasad-sawantdesai Awaiting requested review from prasad-sawantdesai prasad-sawantdesai is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@munechika-koyo @olivhoenen