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 support for non-ellipsis open date windows #103

Merged

Conversation

moradology
Copy link
Collaborator

@moradology moradology commented Nov 16, 2021

STAC Pydantic currently supports open datetime windows with ellipsis ("DATETIME/.." or "../DATETIME") but it appears as though it ought to also support datetimes with non-ellipsis open windows too. Such windows have the form "/DATETIME" or "DATETIME/". This PR corrects that oversight.

@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2021

Codecov Report

Merging #103 (2f7d1d5) into master (cdd0550) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
+ Coverage   96.19%   96.21%   +0.01%     
==========================================
  Files          20       20              
  Lines         473      475       +2     
==========================================
+ Hits          455      457       +2     
  Misses         18       18              
Flag Coverage Δ
unittests 96.21% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
stac_pydantic/api/search.py 96.15% <100.00%> (ø)
stac_pydantic/shared.py 100.00% <0.00%> (ø)
stac_pydantic/links.py 98.24% <0.00%> (+0.03%) ⬆️

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 cdd0550...2f7d1d5. Read the comment docs.

@moradology moradology force-pushed the feature/non-ellipsis-open-date-windows branch from e90090c to 258e7ed Compare November 16, 2021 01:16
Comment on lines 111 to 116
if value == "..":
dates.append(value)
continue
elif value == "":
dates.append("..")
continue

Choose a reason for hiding this comment

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

I think this might be clearer as:

Suggested change
if value == "..":
dates.append(value)
continue
elif value == "":
dates.append("..")
continue
if value == ".." or value == "":
dates.append(value)
continue

I don't know if this is a regular thing to do but I tend to interpret the branches as different behaviors that should occur. Naturally feel free to take or leave this one

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems sensible to me!

@moradology moradology merged commit 4ad4577 into stac-utils:master Nov 22, 2021
@moradology moradology deleted the feature/non-ellipsis-open-date-windows branch November 22, 2021 15:56
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

4 participants