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

feature updates for tgocassisstitch #5162

Merged
merged 16 commits into from
Aug 22, 2023
Merged

feature updates for tgocassisstitch #5162

merged 16 commits into from
Aug 22, 2023

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.

closed #5132 in favor of opening this as to have a cleaner history and branch.

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.

@acpaquette
Copy link
Collaborator

@antonhibl Apologies for taking some time to get to this. Looks like it could use a fix for the changelog conflicts. The commit history here is also very messy. If we could do a git rebase -i HEAD~33, I think we could get it down to a handful of commits. I can help with that as well

Copy link
Collaborator

@acpaquette acpaquette left a comment

Choose a reason for hiding this comment

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

Couple things to address outside of the re-base but overall looks good and tests are passing!

isis/tests/FunctionalTestsTgocassisstitch.cpp Outdated Show resolved Hide resolved
isis/src/tgo/apps/tgocassisstitch/tgocassisstitch.cpp Outdated Show resolved Hide resolved
@antonhibl antonhibl requested a review from acpaquette August 10, 2023 19:28
Copy link
Collaborator

@acpaquette acpaquette left a comment

Choose a reason for hiding this comment

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

Looks like things are still kinda wonky

isis/tests/FunctionalTestsTgocassisstitch.cpp Outdated Show resolved Hide resolved
isis/tests/TgoCassisModuleTests.cpp Outdated Show resolved Hide resolved
@antonhibl antonhibl requested a review from acpaquette August 14, 2023 17:06
@antonhibl
Copy link
Contributor Author

@acpaquette latest commits should have addressed the issues with dashes and I have tested locally to ensure they pass

Copy link
Contributor Author

@antonhibl antonhibl left a comment

Choose a reason for hiding this comment

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

this should be ready for re-review @acpaquette

@antonhibl antonhibl dismissed acpaquette’s stale review August 17, 2023 17:44

outdated, should be addressed

added suffix and prefix capabilities in tgocassisstitch

    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.

minor fixes

typo

more typos

close to done, one error left

    need to figure out a replacement for ui.IsOptionSet as that is not actually
    a function. I need another way to test CLLI args.

final fixes to implement ui changes

fixed

    now employs a boolean flag to decide if the name parsed with out is a suffix
    or prefix, it defaults to a prefix.

adding some tests to confirm output runs as expected.
…bility

    it also maintains original value names so older scripts and pipelines will
    retain functionality.

adds another optional arg, cubename

    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.
removed debugging code

added changelog notes to xml for tgocassisstitch
added cubename argument logic

typo in else statement
removed faulty/old tests based on makefiles

fixed single and multi frame tests for tgocassisstitch

remaining tests fixed, one on EFS to check with jenkins.

removed debug lines

updated to remove nil randomly appearing in tests

modified first stitch test to use fallback naming behaviour.
…nches

removing redundant lines from 8.0 release

addresses prefix and filename convention topical issues
added details to changelog

small typo in link
…bility

    it also maintains original value names so older scripts and pipelines will
    retain functionality.
changelog fixes

removed typo
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@acpaquette acpaquette left a comment

Choose a reason for hiding this comment

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

Sweet, looks good!

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.

2 participants