Skip to content

Conversation

@zigaLuksic
Copy link
Collaborator

Closes #498

@ColinMoldenhauer is this sufficient for your needs? I would also like to add you as a co-author (since the idea and solution suggestion are yours), but I'd need an e-mail under which to add you.

@zigaLuksic zigaLuksic requested a review from jgersak November 14, 2022 12:27
assert all(timestamp.tzinfo is not None for timestamp in timestamps)

def test_get_available_timestamps_custom_filtration(self):
"""Chekcs that the custom filtration works as intended."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Checks

Copy link
Contributor

@ColinMoldenhauer ColinMoldenhauer 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 to me, thank you for the quick action!

@ColinMoldenhauer
Copy link
Contributor

Closes #498

@ColinMoldenhauer is this sufficient for your needs? I would also like to add you as a co-author (since the idea and solution suggestion are yours), but I'd need an e-mail under which to add you.

As an e-mail, you can use colin.moldenhauer [at] posteo.de, thank you!

Co-authored-by: Colin Moldenhauer  <colin.moldenhauer@posteo.de>
@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2022

Codecov Report

Base: 83.60% // Head: 83.58% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (1a755cf) compared to base (05990cb).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #500      +/-   ##
===========================================
- Coverage    83.60%   83.58%   -0.02%     
===========================================
  Files           60       60              
  Lines         5683     5685       +2     
===========================================
+ Hits          4751     4752       +1     
- Misses         932      933       +1     
Impacted Files Coverage Δ
io/eolearn/io/sentinelhub_process.py 87.18% <100.00%> (+0.09%) ⬆️
core/eolearn/core/extra/ray.py 92.68% <0.00%> (-2.44%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jgersak
Copy link
Contributor

jgersak commented Nov 15, 2022

Code looks good.
I would just add test where we would check sth. like:

len(set(timestamp)) == len(timestamp filtration) != len(timestamp))

and make doc strings unified whit existing doc strings.

@zigaLuksic zigaLuksic merged commit 966ab93 into develop Nov 15, 2022
@zigaLuksic zigaLuksic deleted the feat/custom-timestamp-filtration branch November 15, 2022 08:36
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.

5 participants