New release? #60
Comments
|
Hello! Thanks for you interest. |
|
I've have been using it locally, but I would not say that the usage of fels in my workflow is comprehensive, nor have I 100% verified that there isn't some mistake. I did test it to the extent that it passes the tests on the CI, but I don't think what exists is comprehensive. To this end, I wrote another small PR #61 that adds two tests that verify that command line and python API usage produces the same results. I threw a few variations at it, but didn't add in everything that I could of. But if there is anything else you think should be tested, it probably makes sense to have that tested on the CI, so a green dashboard gives confidence that it is ok to release a new version. |
|
Thanks a lot for this! I was looking at opened bugs and remembered we had this #56 still to solve. Do you use windows? |
|
It looks like #56 is more of an issue with fiona / gdal on windows than it is with this package. If you install it from pip, without installing other dependencies first, then they will also be installed from pip. I imagine this has issues on windows, where you probably want to install all the tools that depend on geos (fiona, shapely, gdal) from conda. I don't think there is anything to do in this repo on that. You just have to be careful to install those conda dependencies first. (really the fiona maintainers should release win32 wheels on pypi). There does seem to be another issue. The master dashboards are failing. The specific query results that I got on my machine on those tests are coming back different now. This happened once on my machine, but I thought I made a typo. Seems like there might be some actual non-determinism under the hood. On the latest master dashboards the expected vs actual L8 results I'm getting are: I just deleted my cache directory and re-ran it again, and again I got different results. Something fishy is going on. For landsat I just got: Is it possible that the results for a query with a specific spacetime location in 2015 would be changing? |
|
This is very weird indeed. I have two versions of the cache on my machine (one I made yesterday and one I made today) and they consistently reproduce the same different results. Failing one from today: Passing one from yesterday: It is true that the But would the URL Note, this goes beyond the SQL cache, because the behavior persists even when I fallback to the CSV parsing method. |
|
I think I found the bug. The issue is that I had the flag def sort_url_list(cc_values, all_acqdates, all_urls):
"""Sort the url list by increasing cc_values and acqdate."""
cc_values = sorted(cc_values)
all_acqdates = sorted(all_acqdates, reverse=True)
all_urls = [x for (y, z, x) in sorted(zip(cc_values, all_acqdates, all_urls))]
urls = []
for url in all_urls:
urls.append('http://storage.googleapis.com/' + url.replace('gs://', ''))
return urlsThe following does not work as described. The first two sorts destroy any alignment between the three input lists, thus the all_urls sort command just returns the urls in roughly whatever order they were given in. That is the cause of the bug. I will fix it in a patch. New implementation looks like this: def sort_url_list(cc_values, all_acqdates, all_urls):
"""
Sort the url list, first by ascending cc_values, and then by descending
acqdate. Also replaces the gs:// prefix with the google api http prefix.
Args:
cc_values (List[float]): cloud cover for each item
all_acqdates (List[datetime.datetime]): datetime for each item
all_urls (List[str]): url for each item
Returns:
List[str]: sorted and modified urls
Example:
>>> from fels.utils import * # NOQA
>>> cc_values = [2.11, 1.85, 18.51, 2.85, 3.92, 18.32]
>>> all_acqdates = [datetime.datetime(2015, 3, 31, 0, 0),
>>> datetime.datetime(2015, 6, 19, 0, 0),
>>> datetime.datetime(2015, 2, 27, 0, 0),
>>> datetime.datetime(2015, 1, 26, 0, 0),
>>> datetime.datetime(2015, 3, 15, 0, 0),
>>> datetime.datetime(2015, 6, 3, 0, 0)]
>>> all_urls = ['gs://test_url_{}'.format(i) for i in range(len(cc_values))]
>>> sorted_urls = sort_url_list(cc_values, all_acqdates, all_urls)
>>> print('sorted_urls = {}'.format(ub.repr2(sorted_urls, nl=1)))
sorted_urls = [
'http://storage.googleapis.com/test_url_1',
'http://storage.googleapis.com/test_url_0',
'http://storage.googleapis.com/test_url_3',
'http://storage.googleapis.com/test_url_4',
'http://storage.googleapis.com/test_url_5',
'http://storage.googleapis.com/test_url_2',
]
"""
# For implementation clarity, table-like list of dictionary rows
rows = [
{'cc': cc, 'date': date, 'url': url}
for cc, date, url in zip(cc_values, all_acqdates, all_urls)
]
# First group and sort by ascending cloudcover
cc_to_rows = ubelt.group_items(rows, key=lambda row: row['cc'])
cc_to_rows = ubelt.sorted_keys(cc_to_rows)
sorted_urls = []
for cc, group in cc_to_rows.items():
# Then within each group, sort by descending date
group = sorted(group, key=lambda row: (row['date'], row['url']), reverse=True)
for row in group:
url = row['url']
new_url = 'http://storage.googleapis.com/' + url.replace('gs://', '')
sorted_urls.append(new_url)
return sorted_urlsIt's not as efficient as it could be, but it's not inefficient by any stretch, and I think the clarity of the implementation is better, which is important considering this function was buggy. (It's also not clear to me how to sort by a tuple where one item is ascending and another is descending when I can't easily negate one of the terms, so I like this grouping solution better). Hopefully it is clear what the ubelt functions are doing. (they are documented: https://ubelt.readthedocs.io/en/latest/ubelt.html#ubelt.group_items) |
Wondering if there are plans to release the latest code on pypi? Looks like the version in
__init__.pyneeds to be bumped. Current version on pypi is 1.4.0 (https://pypi.org/project/FeLS/), which is what is also reported in__init__.py, but the current code is newer than that.I made a PR that bumps the version: #59
The text was updated successfully, but these errors were encountered: