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

feat: external forcing converter cli (unst-8490) #741

Open
wants to merge 15 commits into
base: feature/unst-8490-tim-to-bc-boundary-condition
Choose a base branch
from

Conversation

MAfarrag
Copy link
Contributor

@MAfarrag MAfarrag commented Feb 24, 2025

@MAfarrag MAfarrag changed the base branch from main to feature/unst-8490-tim-to-bc-boundary-condition February 24, 2025 10:32
@MAfarrag MAfarrag added the extforce external forcing file conversion label Feb 25, 2025
…ub.com:Deltares/HYDROLIB-core into feature/unst-8490-ext-forcing-converter-cli
Copy link
Member

@arthurvd arthurvd left a comment

Choose a reason for hiding this comment

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

See small comments

from pathlib import Path

from hydrolib.core import __version__
from hydrolib.tools.ext_old_to_new.main_converter import (
Copy link
Member

Choose a reason for hiding this comment

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

As discussed: let's have one and the same name for both the tools/ subdirectory "ext_old_to_main" and the new one you introduced in pyproject.toml: "extforce-convert" <-- let's use your name everywhere.

Comment on lines +65 to +77
backup_group.add_argument(
"--backup",
"-b",
action="store_true",
default=True,
help="Create a backup of each file that will be overwritten.",
)
backup_group.add_argument(
"--no-backup",
dest="backup",
action="store_false",
help="Do not create a backup of each file that will be overwritten.",
)
Copy link
Member

Choose a reason for hiding this comment

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

I just found that argparse allows you to do this with one single call:
class argparse.BooleanOptionalAction

(But check whether the help docs work out well for both the --backup and --no-backup option.)

…ub.com:Deltares/HYDROLIB-core into feature/unst-8490-ext-forcing-converter-cli
…feature/unst-8490-tim-to-bc-boundary-condition' of github.com:Deltares/HYDROLIB-core into feature/unst-8490-ext-forcing-converter-cli
…com:Deltares/HYDROLIB-core into feature/unst-8490-ext-forcing-converter-cli
…ub.com:Deltares/HYDROLIB-core into feature/unst-8490-ext-forcing-converter-cli
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extforce external forcing file conversion
Projects
Status: Review in progress
Development

Successfully merging this pull request may close these issues.

2 participants