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

allow methods to use de_train_h5ad instead of de_train. #14

Merged
merged 5 commits into from
May 6, 2024
Merged

Conversation

rcannood
Copy link
Member

@rcannood rcannood commented May 3, 2024

Describe your changes

Checklist before requesting a review

  • I have performed a self-review of my code

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality (new method, new metric, ...)
    • Major changes
    • Minor changes
    • Bug fixes
  • Proposed changes are described in the CHANGELOG.md

  • CI Tests succeed and look good!

@rcannood rcannood requested a review from szalata May 3, 2024 07:59
Copy link
Collaborator

@szalata szalata left a comment

Choose a reason for hiding this comment

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

We get an error in
def set_par_values(config) -> None:

due to the resources path based on par argument that isn't necessary.

❯ ./scripts/add_a_method.sh
[warning] --task looks like a parameter but is not a defined parameter and will instead be treated as a positional argument. Use --help to get more information on the parameters.
Check inputs
Check language
Check API file
Read API file
Create output dir
Create config
Create script
Traceback (most recent call last):
  File "/viash_automount/var/folders/98/9ygjwkpd12d1vxydfprn1st00000gp/T//viash-run-create_component-CTkCHC.py", line 358, in <module>
    main(par)
  File "/viash_automount/var/folders/98/9ygjwkpd12d1vxydfprn1st00000gp/T//viash-run-create_component-CTkCHC.py", line 342, in main
    set_par_values(api)
  File "/viash_automount/var/folders/98/9ygjwkpd12d1vxydfprn1st00000gp/T//viash-run-create_component-CTkCHC.py", line 194, in set_par_values
    value = f'resources_test/{par["task"]}/pancreas/{key_strip}.h5ad'
KeyError: 'task'
Files and logs are stored at '/tmp/viash_create_component12384494734812512647'

@szalata
Copy link
Collaborator

szalata commented May 4, 2024

for the issue in def set_par_values(config) -> None:

I think we could replace with:

    # find value
    if arg["type"] != "file":
      value = arg.get("default", arg.get("example", "..."))
    elif key == "de_train":
      value = "resources/neurips-2023-kaggle/de_train.parquet"
    elif key == "de_train":
      value = "resources/neurips-2023-kaggle/de_train.h5ad"
    elif key == "id_map":
      value = "resources/neurips-2023-kaggle/id_map.csv"
    else:
      key_strip = key.replace("output_", "")
      value = f'{key_strip}.h5ad'

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rcannood could you please confirm whether that removal is ok?

@szalata
Copy link
Collaborator

szalata commented May 4, 2024

after the 2 small changes I commented on above, lgtm!

@rcannood rcannood merged commit 00b03a0 into main May 6, 2024
6 checks passed
@rcannood rcannood deleted the add-h5ad branch May 6, 2024 12:59
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