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

added suffix and prefix capabilities in tgocassisstitch #5132

Closed
wants to merge 89 commits into from

Conversation

antonhibl
Copy link
Contributor

Description

addresses #5125, this allows the user to optionally use either the prefix or
suffix option when stitching frames together, just use OUTPUTSUFFIX instead
of OUTPUTPREFIX to use it.

Related Issue

#5125

How Has This Been Validated?

Tested locally and works by passing in either a OUTPUTSUFFIX or OUTPUT PREFIX.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation change (update to the documentation; no code change)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Infrastructure change (changes to things like CI or the build system that do not impact users)

Checklist:

  • I have read and agree to abide by the Code of Conduct
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added myself to the .zenodo.json document.
  • I have added any user impacting changes to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

  • I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

antonhibl and others added 26 commits February 23, 2023 11:11
I added the following changes to create_rclone_arguments:

- I added a default filter flag --filter with the value of exclude_string

- I added a check for any additional --include and --exclude flags that may have
  been passed in by the user.

- I added logic to merge any additional --include and --exclude flags with the
  default --filter flag. The include and exclude patterns are concatenated with
  the default exclude_string separated by comma.

- I removed the f"--exclude={exclude_string}" from extra_args list, since the
  default exclude_string is now included in the filter flag

- I added the filter flag to the extra_args list using
  extra_args.extend(filter_args)

All of the above changes ensures that the --include and --exclude flags passed
    in by the user are taken into account while creating extra_args, and also
    the logic will merge these flags with the default filter flag, which is the
    recommended way as per the rclone docs.
along with a few other changes:

   + I added a check for any --filter flag provided by the user, if present it
    will use it and ignore the default filter flag. Otherwise, it will use the default
    filter flag. This is done to take into account if the user has provided any
    specific filter flag, and it will honor the user's intention of providing
    the filter flag.

   + I added a check for any additional --include and --exclude flags passed in
    by the user and merge them with the filter flag. This is to take into
    account any specific include/exclude patterns that the user wants to apply,
    and merge them with the default filter flag.

   + I added a "+" at the end of the filter string if the user has specified an
    --include flag. This simulates the behavior of --include where it includes
    any patterns specified and excludes everything else.

   + I also added a check for filter_args_provided and if provided, it will use
    this flag, else it will use the default filter flag and merge any additional
    include/exclude flags to it.

There is also a test in the pytest file to check if the filter logic works as
expected, run using `pytest`.
**The first test that was added is `test_rclone_with_auth`**
    This test is designed to check the behavior of the `rclone` function when it
    is called with an `auth` parameter. This test checks that the `rclone`
    function properly passes the "auth" parameter to the underlying subprocess
    call.

 **The second test that was added is `test_create_rclone_args_with_no_kwargs`**
    This test is designed to check the behavior of the `create_rclone_args`
    function when it is called with no keyword arguments. This test checks that
    the `create_rclone_args` function properly handles the case when it is
    called with no keyword arguments and returns the correct list of arguments
    to be passed to the underlying subprocess call

 **The third test that was added is `test_file_filtering_with_hidden_files`**
    This test is designed to check the behavior of the `file_filtering` function
    when it is called with hidden files in the specified directory. This test
    checks that the `file_filtering` function properly filters out hidden files
    and only returns the non-hidden files in the specified directory.

I have tested to confirm these all run and pass on my machine.
**added `test_rclone` test**
    This test mocks the `subprocess.Popen` function and checks that the output
    of the rclone function matches the expected output when the function is
    invoked with the arguments `lsf`, `test`, `["-l", "-R", "--format", "p",
    "--files-only"]`, `True`, and `True`.

**added `test_rclone_unknown_exception` test**
    This test mocks the `subprocess.Popen` function and checks that the `rclone`
    function raises an exception when an unknown exception is encountered. This
    test uses a mocked class that raises an exception when it is initialized.

I have tested and confirmed these to work on my system.
    fixed how filters are input, patterns still aren't working I believe I need
    to look at their patterns and ensure the adhere to rclone documentation
    syntactical instructions.
    addresses DOI-USGS#5125, this allows the user to optionally use either the prefix or
    suffix option when stitching frames together, just use OUTPUTSUFFIX instead
    of OUTPUTPREFIX to use it.
@antonhibl antonhibl added enhancement New feature or request in progress doing the things labels Feb 24, 2023
@antonhibl antonhibl self-assigned this Feb 24, 2023
    need to figure out a replacement for ui.IsOptionSet as that is not actually
    a function. I need another way to test CLLI args.
    now employs a boolean flag to decide if the name parsed with out is a suffix
    or prefix, it defaults to a prefix.
…bility

    it also maintains original value names so older scripts and pipelines will
    retain functionality.
    lets the user specify an optional cubename parameter which replaces the
    timestamp style naming for the output stitched cube. If the parameter is not
    given it defaults to the timestamp style naming convention for retaining
    existing functionality.
…bility

    it also maintains original value names so older scripts and pipelines will
    retain functionality.
@acpaquette
Copy link
Collaborator

@antonhibl Good on building now, only one more to go! https://jenkins-hsts.prod-asc.chs.usgs.gov/job/ISIS/view/change-requests/job/PR-5132/

The DemCube test failure is something else you don't have to worry about

@antonhibl antonhibl closed this Mar 28, 2023
@antonhibl
Copy link
Contributor Author

closed this in favor of opening #5162 with a cleaner commit history and branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress doing the things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add filename prefix option for tgocassisstitch
5 participants