Skip to content

Use pydantic field to automate argument building and parsing.#162

Merged
YooSunYoung merged 20 commits intomainfrom
pydantic-field
Nov 20, 2025
Merged

Use pydantic field to automate argument building and parsing.#162
YooSunYoung merged 20 commits intomainfrom
pydantic-field

Conversation

@YooSunYoung
Copy link
Copy Markdown
Member

Continued from #160

Comment thread src/ess/nmx/_executable_helper.py Outdated
return getattr(args, field_name)


class CommandArgument(BaseModel):
Copy link
Copy Markdown
Member

@SimonHeybrock SimonHeybrock Nov 20, 2025

Choose a reason for hiding this comment

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

Not sure why this needs to be used as a base model. Can't the same be done with a free function such that models do not need to inherit? That would be much cleaner, as you would avoid forcing workflows to know about argument parsing.

Copy link
Copy Markdown
Member Author

@YooSunYoung YooSunYoung Nov 20, 2025

Choose a reason for hiding this comment

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

[WIP] Fixed it!
Sure.
I thought it would be eaiser this way if we want to be explitic about turning them into command line argument or not.
We can take care of that case in the free function too I guess?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From my point of views the pydantic models defining the parameters might be useful for multiple purposes, e.g., to create widgets. It would thus be a more versatile design if it would work without inheritance.

Comment thread src/ess/nmx/_executable_helper.py Outdated
output_file=args.output_file,
compression=Compression[args.compression],
)
class TOAUnit(Enum):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider StrEnum, since we now have Python 3.11

Comment thread src/ess/nmx/_executable_helper.py Outdated
Comment on lines +175 to +186
min_toa: int = Field(
title="Minimum Time of Arrival",
description="Minimum time of arrival (TOA) in [toa_unit].",
default=0,
)
max_toa: int = Field(
title="Maximum Time of Arrival",
description="Maximum time of arrival (TOA) in [toa_unit].",
default=int((1 / 14) * 1_000),
)
toa_unit: TOAUnit = Field(
title="Maximum Time of Arrival",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A bunch of titles are off here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh no...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed it! But which titles you mean? It was only toa_unit...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right!

Comment thread src/ess/nmx/_executable_helper.py Outdated
Comment on lines +21 to +24
# Supported annotation for command arguments:
# - Atomic types: int, float, str, bool, enum.StrEnum, Literal
# - Optional[AtomicType]
# - List[AtomicType], Tuple[AtomicType, ...], Set[AtomicType]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe this belong into the docstring of add_args_from_pydantic_model?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added docstring to the helper functions including these lines.

@YooSunYoung YooSunYoung changed the base branch from multi-input-files to main November 20, 2025 12:49
@YooSunYoung YooSunYoung merged commit 5c8c7c5 into main Nov 20, 2025
4 checks passed
@YooSunYoung YooSunYoung deleted the pydantic-field branch November 20, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants