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

not enough values to unpack (expected 4, got 0) in outside_the_boundaries_of #209

Open
rez0n opened this issue Nov 7, 2023 · 15 comments
Open

Comments

@rez0n
Copy link

rez0n commented Nov 7, 2023

Hi,
I faced with some rare issue in timezonefinder, it throwing not enough values to unpack (expected 4, got 0) exception randomly and rarely. I have no ideas about reproducing of this because I ran it for all coordinates in the database multiple times and have no luck to catch exception.
Submitting this report just in case if author have any ideas.

Timezonefinder version: 6.2.0
Python 3.11

Usage example:

tf = TimezoneFinder()
tz = tf.timezone_at(lng=float(longitude), lat=float(latitude))

CleanShot 2023-11-07 at 22 35 29@2x

@jannikmi
Copy link
Owner

jannikmi commented Nov 8, 2023

Thanks for reporting this. It would be best to unit test the respective functions for all possible values. Unfortunately I don't have enough time to implement this. Can anyone help?

@jannikmi
Copy link
Owner

jannikmi commented Feb 2, 2024

Hello @rez0n, are you sure the error originates from the line in the screenshot?

@jannikmi
Copy link
Owner

jannikmi commented Feb 2, 2024

I added unit tests for the highlighted functions. The tests are passing. Additional unit tests are required to rule out possible bugs.

@rez0n can you provide an example input to reproduce this? It seems very strange that the error cannot be reproduced even if you repeat your experiments. The code of timezonefinder should be deterministic, so it should either always or never throw an error for identical input.

@rez0n
Copy link
Author

rez0n commented Feb 16, 2024

Hi @jannikmi
Sorry for late response.

are you sure the error originates from the line in the screenshot?

As you can see, it is Sentry.io report. It is always correct for me.

can you provide an example input to reproduce this?

Unfortunately no, as I said in the initial message I ran script for entire database and no any value did not triggered this error.
It is seems something very low level, maybe? On screenshot you can see x and y variables, maybe it is something what could help to debug this?

@kuanb
Copy link

kuanb commented Mar 21, 2024

Just chiming in that I am running into this now. In a running service I see the following logged:

cannot find timezone for: 120.347287,22.598127; ERROR: not enough values to unpack (expected 4, got 0)

When I run the same locally, it executes fine:

_coordinates = "120.347287,22.598127"

parsed_ll = [float(x) for x in _coordinates.split(",")]
tzid = TZ_FINDER.timezone_at(lng=parsed_ll[0], lat=parsed_ll[1])
assert tzid == 'Asia/Taipei'

Similar to the above poster, this occurs seemingly at random and is not reproducible locally. Will continue to investigate and report back here if I am able to discern further...

@jannikmi
Copy link
Owner

Thanks for reporting this!
Could you perhaps add small random noise to the floating point coordinates?

@chboe
Copy link

chboe commented Jun 3, 2024

@kuanb did you figure out what the problem was? I am also facing this issue at random.

@jannikmi
Copy link
Owner

jannikmi commented Jun 4, 2024

@chboe can you perhaps help to reproduce it and add some test cases?

@chboe
Copy link

chboe commented Jun 4, 2024

@jannikmi It happens totally at random. Very odd behaviour. I tried adding some random noise and querying a second time. Will see if this solves my issue.

It doesn't even seem to be a boundary issue since all my test cases run very clearly inside timezone boundaries

@jannikmi
Copy link
Owner

jannikmi commented Jun 4, 2024

Thanks for the report.

And you encountered the error while running the query for statically defined test cases (coordinates)? That would inply that the behaviour of Timezonefinder is not deterministic, which would be very surprising for me.

@chboe
Copy link

chboe commented Jun 4, 2024

Not well-defined test-cases but some code running in production that occasionally logged and error in datadog. When I then re-queryed with the same lat-lon it went fine - both in production and locally.

@jannikmi
Copy link
Owner

jannikmi commented Jun 4, 2024

Might the different behaviour come from different floating point precisions?

@ringsaturn
Copy link
Contributor

If users face this issue, it's possible to make a minor change to capture what _fromfile actually returns.

From the example, we can see that 'in_memory' is False in the TimezoneFinder by default, so the data is returned by NumPy's fromfile.

I suggest adding these changes to get_polygon_boundaries:

    def get_polygon_boundaries(self, poly_id: int) -> Tuple[int, int, int, int]:
        """returns the boundaries of the polygon = (lng_max, lng_min, lat_max, lat_min) converted to int32"""
        poly_max_values = getattr(self, POLY_MAX_VALUES)
        poly_max_values.seek(4 * NR_BYTES_I * poly_id)
-       xmax, xmin, ymax, ymin = self._fromfile(
+       file_result = self._fromfile(
            poly_max_values,
            dtype=DTYPE_FORMAT_SIGNED_I_NUMPY,
            count=4,
        )
+       xmax, xmin, ymax, ymin = file_result
        return xmax, xmin, ymax, ymin

Then we can get the file_result in platforms like Sentry.

Or inside timezonefinder, add a try/except to capture the results and add them to the exception messages.


The code inside NumPy's fromfile will be hard to debug: https://github.com/numpy/numpy/blob/6f428f2e0cbd2b809ce9ba76a531e01d6c983b9c/numpy/_core/records.py#L933-L939

@jannikmi
Copy link
Owner

Thanks for the suggestion. Could you open a PR with these changes?

@jannikmi
Copy link
Owner

jannikmi commented Dec 2, 2024

@rez0n, @kuanb, @chboe, @ringsaturn in the lastest version 6.5.7 I improved the error handling and logging to catch the Value Error. Please update your timezonefinder version and report back when you hit this error again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants