Skip to content

Conversation

@jreiberkyle
Copy link
Contributor

Related Issue(s):

Closes #956

Proposed Changes:

For inclusion in changelog (if applicable):

  1. Separate data api item type validation from orders api item type validation, add SkySatVideo as a valid data api item type

Not intended for changelog:

Diff of User Interface

Old behavior:

planet-client-python$> planet data search --help
Usage: planet data search [OPTIONS] ITEM_TYPES

...

  Valid entries for ITEM_TYPES: MYD09GA|MOD09GA|Sentinel1|Landsat8
  L1G|PSScene|SkySatCollect|MYD09GQ|PSOrthoTile|MOD09GQ|REScene|Sentinel2L1C|S
  kySatScene|REOrthoTile
planet data search skysatvideo
Usage: planet data search [OPTIONS] ITEM_TYPES
Try 'planet data search --help' for help.

Error: Invalid value for 'ITEM_TYPES': item_type - 'skysatvideo' is not one of 'REOrthoTile', 'REScene', 'MOD09GA', 'Landsat8L1G', 'SkySatScene', 'MYD09GQ', 'MYD09GA', 'MOD09GQ', 'PSScene', 'SkySatCollect', 'Sentinel1', 'PSOrthoTile', 'Sentinel2L1C'.

New behavior:

SkySatVideo listed as a valid entry for ITEM_TYPE

planet-client-python$> planet data search --help
Usage: planet data search [OPTIONS] ITEM_TYPES

...

  Valid entries for ITEM_TYPES: MYD09GA|MOD09GA|Sentinel1|SkySatVideo|Landsat8
  L1G|PSScene|SkySatCollect|MYD09GQ|PSOrthoTile|MOD09GQ|REScene|Sentinel2L1C|S
  kySatScene|REOrthoTile
planet data search skysatvideo
<VALID_SKYSATVIDEO_ITEM_DESCRIPTIONS>

PR Checklist:

  • [] This PR is as small and focused as possible
  • [] If this PR includes proposed changes for inclusion in the changelog, the title of this PR summarizes those changes and is ready for inclusion in the Changelog.
  • [] I have updated docstrings for function changes and docs in the 'docs' folder for user interface / behavior changes
  • [] This PR does not break any examples or I have updated them

(Optional) @mentions for Notifications:

@jreiberkyle jreiberkyle requested review from cholmes and sgillies May 18, 2023 20:25
@jreiberkyle jreiberkyle changed the base branch from main to maint-2.0 May 18, 2023 20:25
Copy link
Contributor

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

@jreiberkyle I made a suggestion to use less code. 👍 on joining the sets like you've done.

def validate_data_item_type(item_type):
'''Item types supported by the data api.'''
# This is a quick-fix for gh-956
return _validate_field(item_type, get_data_item_types(), 'item_type')
Copy link
Contributor

Choose a reason for hiding this comment

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

@jreiberkyle since get_data_item_types() below is essentially a one line function, how about this instead? Let's avoid creating new functions when they're not strictly required at the moment.

Suggested change
return _validate_field(item_type, get_data_item_types(), 'item_type')
return _validate_field(item_type, get_item_types() | {'SkySatVideo'}, 'item_type')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohhh wait, I was going to say yes absolutely but the data CLI uses get_data_item_types() to list the valid item types in the help. So removing get_data_item_types() would require the same one-liner to be added to planet/cli/data.py as well. Introducing get_date_item_types() was chosen to keep the "hacktastic" code in one place within specs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but looking at the code, there are a lot of opportunities to get rid of short-term variables and extra lines of code. I did what I could while keeping get_data_item_types()

@jreiberkyle jreiberkyle merged commit 57aa7e7 into maint-2.0 May 19, 2023
@jreiberkyle jreiberkyle deleted the skysatvideo-956 branch May 19, 2023 19:46
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.

sdk does not support data api skysatvideo item type

3 participants