Updated CLI options for new API

scap3yvt commented 3 months ago

Fixes #849, #851

Proposed Changes

updated cli options for the new cli

Checklist

[x] CONTRIBUTING guide has been followed.
[x] PR is based on the current GaNDLF master .
[x] Non-breaking change (does not break existing functionality): provide as many details as possible for any breaking change.
[x] Function/class source code documentation added/updated (ensure typing is used to provide type hints, including and not limited to using Optional if a variable has a pre-defined value).
[x] Code has been blacked for style consistency and linting.
[x] If applicable, version information has been updated in GANDLF/version.py.
[x] If adding a git submodule, add to list of exceptions for black styling in pyproject.toml file.
[x] Usage documentation has been updated, if appropriate.
[x] Tests added or modified to cover the changes; if coverage is reduced, please give explanation.
[x] If customized dependency installation is required (i.e., a separate pip install step is needed for PR to be functional), please ensure it is reflected in all the files that control the CI, namely: python-test.yml, and all docker files [1,2,3].

github-actions[bot] commented 3 months ago

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

scap3yvt commented 2 months ago

I think this should be ready, @VukW!

VukW commented 2 months ago

Hi @scap3yvt ! I don't know why CI didn't trigger pytest after last merge, but for me locally tests fail, in particular these two: https://github.com/mlcommons/GaNDLF/blob/new-apis_v0.1.0-dev__849-standardize_cli_options/testing/entrypoints/test_preprocess.py https://github.com/mlcommons/GaNDLF/blob/new-apis_v0.1.0-dev__849-standardize_cli_options/testing/entrypoints/test_patch_miner.py

that's reasonable as they use old param names. Also, tests on old way scripts fail also, say here (and same fore patch_miner)

You can check tests locally via pytest ./testing/entrypoints/test_preprocess.py and pytest ./testing/entrypoints/test_patch_miner.py

sarthakpati commented 2 months ago

I am curious/concerned why the tests are not getting triggered...

VukW commented 2 months ago

@sarthakpati I checked the workflow, it's triggered only on PR to master:

on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]

(as well as codacy, codeql-analysis, devcontainer, mlcube-test, openfl-test, ossar-analysis workflows)

sarthakpati commented 2 months ago

Ah thanks! I am going to put a PR about that now.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.41%. Comparing base (2ed393d) to head (fda8c6d).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## new-apis_v0.1.0-dev #853 +/- ## ==================================================== Coverage 94.41% 94.41% ==================================================== Files 158 158 Lines 9345 9345 ==================================================== Hits 8823 8823 Misses 522 522 ``` | [Flag](https://app.codecov.io/gh/mlcommons/GaNDLF/pull/853/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mlcommons) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/mlcommons/GaNDLF/pull/853/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mlcommons) | `94.41% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mlcommons#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mlcommons / GaNDLF