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

Add writing a cube out to topds4 #4241

Merged
merged 7 commits into from
Jan 7, 2021
Merged

Conversation

kberryUSGS
Copy link
Contributor

@kberryUSGS kberryUSGS commented Jan 6, 2021

Description

Converts cubeatt to a callable and then uses it in topds4 to generate an output cube.

Related Issue

PDS4 work.

Motivation and Context

So that topds4 can output a cube in addition to a label.

How Has This Been Tested?

Screenshots (if appropriate):

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)

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.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • 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.

// NEED TO WRITE AND CLOSE THE OUTPUT FILE BEFORE RENDERING SO THE FILE SIZE CALLBACK CAN GET THE FINAL FILE SIZE

// Need a better solution for this.
QString outputCubePath = outputFile+".cub";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to handle one output filepath indicated by "TO" vs. two output files: the label and the cube itself. tgocassisrdrgen uses the one input for both and appends ".xml" or ".img" as needed. Thoughts for this application?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would have to specify the xml label and then strip that off to get the cube name. Ideally it would check if the extension is .xml, .cub, or neither. Then it will strip and add extensions as needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Most if not all of the apps that deal with .img .xml .lbl, ... ask for the label file then change the extension to get at the other files.

EXPECT_NEAR(pixelGroup["Base"][0].toDouble(), -0.003952569, 0.00001);
EXPECT_NEAR(pixelGroup["Multiplier"][0].toDouble(), 0.003952569, 0.00001);

// Test the DNs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to add this part of the tests still.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would pull the Cube statistics and check that the min/max values are between the specified values in the cubeatt call. The small cube fixture has values ranging from 0 to 1000, so I would pick something other than 0:1 for your range in this test. Maybe something like 100:460 that will cover about half of the pixels?

Copy link
Contributor

@jessemapel jessemapel left a comment

Choose a reason for hiding this comment

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

Conversion looks good. I added some comments about how to improve the tests and how to check DNs

isis/tests/FunctionalTestsCubeatt.cpp Outdated Show resolved Hide resolved
EXPECT_NEAR(pixelGroup["Base"][0].toDouble(), -0.003952569, 0.00001);
EXPECT_NEAR(pixelGroup["Multiplier"][0].toDouble(), 0.003952569, 0.00001);

// Test the DNs
Copy link
Contributor

Choose a reason for hiding this comment

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

I would pull the Cube statistics and check that the min/max values are between the specified values in the cubeatt call. The small cube fixture has values ranging from 0 to 1000, so I would pick something other than 0:1 for your range in this test. Maybe something like 100:460 that will cover about half of the pixels?

isis/tests/FunctionalTestsCubeatt.cpp Show resolved Hide resolved
isis/tests/FunctionalTestsCubeatt.cpp Outdated Show resolved Hide resolved
isis/tests/FunctionalTestsCubeatt.cpp Outdated Show resolved Hide resolved
ProcessByLine p;

// Should we propagate tables
if(propTables) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems backwards.

Copy link
Contributor Author

@kberryUSGS kberryUSGS Jan 6, 2021

Choose a reason for hiding this comment

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

Good catch! Also, I wrote this weirdly anyway. I've cleaned it up now to just do: p.PropagateTables(propTables);

// NEED TO WRITE AND CLOSE THE OUTPUT FILE BEFORE RENDERING SO THE FILE SIZE CALLBACK CAN GET THE FINAL FILE SIZE

// Need a better solution for this.
QString outputCubePath = outputFile+".cub";
Copy link
Contributor

Choose a reason for hiding this comment

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

Most if not all of the apps that deal with .img .xml .lbl, ... ask for the label file then change the extension to get at the other files.

Copy link
Contributor

@jessemapel jessemapel left a comment

Choose a reason for hiding this comment

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

Looks good, just one issue that's causing other tests to fail.

isis/tests/Fixtures.cpp Outdated Show resolved Hide resolved
@jessemapel
Copy link
Contributor

Also, when we do test conversion we are deleting the old test makefiles

@kberryUSGS kberryUSGS merged commit 5a6586c into DOI-USGS:topds4 Jan 7, 2021
scsides added a commit that referenced this pull request Jan 12, 2021
* Removed compile and deprication warnings. Fixes #4110

* temp add of to pds 4 test

* Initial topds4 app and cmake updates to include inja (#4207)

* Removed compile and deprication warnings. Fixes #4110

* temp add of to pds 4 test

* Inintal topds4 app and cmake updates to include inja

* Delete extra headers

* Fixed inja include

Co-authored-by: ssides <ssides@usgs.gov>

* Added conda-forge inja

* add template file

* Adds PVL to JSON converters (#4212)

* Pvl to JSON conversion

* Renamed file

* Added repeated elements and examples

* Topds4 real parameters (#4223)

* now has some real parameters

* removed inja files

* Add full orex pds4 template (#4233)

* now has some real parameters

* removed inja files

* full orex xml

* add full orex template

* Added callbacks and tests (#4226)

* Adds Xml2Json capability to topds4 branch (#4228)

* Inintal topds4 app and cmake updates to include inja

* Delete extra headers

* Fixed inja include

* Added basic working XMLToJson functionality

* Updated to work with many nested levels of different amounts of repeated tags

* Add support for unsupported xml edge cases

* Add tests that work to test mutliple nested repeated values conversion

* cleanup

* Update to include no-text value cases to work in repeated-value cases

* Update test

* Break up tests

* Fix typoes in comments

* Normalized topds4 and added tests (#4234)

* Removed data source args and added app log

* Added simple tests for topds4

* Moved line

* Added more template elements and callback for file size (#4236)

* now has some real parameters

* removed inja files

* full orex xml

* add full orex template

* More template elements, and file size callback

* Added Extra data source parameters to topds4 (#4239)

* Added extra parameters

* Added duplicate warnings

* Fixed warning

* Add writing a cube out to topds4 (#4241)

* Basic cubeatt conversion

* Add more useful function signature

* Removed hardcoded path

* Update based on feedback

* Add better band test

* Updated to fix failing tests and remove old testfiles

* Add copy of topds4 templates to CMakeLists.txt (#4242)

* Fixed callbacks and doc updates (#4243)

* Fixed callbacks and doc updates

* added some extra docs

* Added original xml label and tests

* Update cubeattribute to include an option to supply CubeAttributeInput directly (#4244)

* Update cubeatt to add cubeattributeinput option. Also add tests for all function signatures available

* Remove unnecessary comments

* Update to use explicit array conversion.

* Remove commented out code

* Update CHANGELOG to include topds4 addition

* Delete osirisrex.xml.tpl

Removed temporary file

* Delete 20190612T090019S776_map_L0pan.xml.full

Remove temporary file

* Delete 20190612T090019S776_map_L0pan.xml.tpl

Remove temp file

* Added mis-deleted file (#4249)

* Update XmlToJson.cpp

* Update topds4.xml

* Update topds4.xml

* Update topds4.xml

* Add setting of virtual bands to topds4 and remove unneeded header file includes

Co-authored-by: ssides <ssides@usgs.gov>
Co-authored-by: Kristin Berry <kberry@usgs.gov>
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.

None yet

3 participants