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

Replace CLI Search Expressions #840

Merged
merged 12 commits into from
Dec 20, 2019
Merged

Replace CLI Search Expressions #840

merged 12 commits into from
Dec 20, 2019

Conversation

omad
Copy link
Member

@omad omad commented Dec 19, 2019

Reason for this pull request

I would like to replace the search expressions used in several datacube command line tools with a more manageable and understandable alternative.

The existing search expressions have confused a few new users, they have grown since initially implemented, but haven't had old options removed. The implementation is also based on PyPEG2, a no longer supported or updated parsing library. Due to it's rarity, this dependency has created several dependency graph issues with datacube in conda-forge.

Proposed changes

  • Remove the PyPEG2 based expression parsing code, and the dependency on PyPEG2
  • Replace with a simpler and more strictly defined implementation based on lark parser
  • Remove the old greater-than and less-than syntax. They were awkward to type, required shell escaping, and seemed to suggest that open ended queries were possible.
  • Remove the in range() syntax, which also required shell escaping, and wasn't clear whether it supported single or multiple dates or only numbers.
Available Search Expressions
  Select datasets using [EXPRESSIONS] to filter by date, product type,
  spatial extents or other searchable fields.

      FIELD = VALUE
      FIELD in DATE-RANGE
      FIELD in [START, END]

  DATE-RANGE is one of YYYY, YYYY-MM or YYYY-MM-DD
  START and END can be either numbers or dates

  FIELD: x, y, lat, lon, time, product, ...

  eg. 'time in [1996-01-01, 1996-12-31]'
      'time in 1996'
      'lon in [130, 140]' 'lat in [-40, -30]'
      product=ls5_nbar_albers

Thanks @uchchwhash for a lot of the hard work!

@omad omad requested a review from jeremyh December 19, 2019 03:34
jeremyh
jeremyh previously approved these changes Dec 19, 2019
Copy link
Contributor

@jeremyh jeremyh left a comment

Choose a reason for hiding this comment

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

Nice, much simpler! 👍

Delete Dataset(s) from the database

Easy if they have nothing derived from them.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Accidental inclusion? (empty stub methods)

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, thanks, yep. I'll clean it up. :)

@coveralls
Copy link

coveralls commented Dec 19, 2019

Coverage Status

Coverage increased (+0.03%) to 88.627% when pulling 8d7c79e on dra/use-larkparser into 8cb0604 on develop.

@omad omad marked this pull request as ready for review December 19, 2019 23:11
@codecov
Copy link

codecov bot commented Dec 19, 2019

Codecov Report

Merging #840 into develop will increase coverage by 0.03%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #840      +/-   ##
===========================================
+ Coverage    88.63%   88.66%   +0.03%     
===========================================
  Files          112      112              
  Lines        10381    10270     -111     
===========================================
- Hits          9201     9106      -95     
+ Misses        1180     1164      -16
Impacted Files Coverage Δ
datacube/index/_api.py 85.71% <100%> (ø) ⬆️
datacube/ui/click.py 84.93% <100%> (+1.81%) ⬆️
datacube/ui/expression.py 77.77% <76.74%> (-6.69%) ⬇️
datacube/drivers/postgres/_fields.py 90.11% <0%> (-0.4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cb0604...8d7c79e. Read the comment docs.


If the product does have Datasets, suggest how to delete the datasets.

"""
Copy link
Member

Choose a reason for hiding this comment

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

more accidental stub commits?

@mergify mergify bot merged commit 19d6dc9 into develop Dec 20, 2019
@mergify mergify bot deleted the dra/use-larkparser branch December 20, 2019 05:05
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.

Replace usage of PyPEG2 with lark-parser
5 participants