Index Error Fix

[gaurav25122000]

Added conversion for float to int for existing indexes while being fetched from mongo database due to pymongo check of indexes being 1,-1 or 2d

[roman-right]

Hi @gaurav25122000
Thank you for the PR! Could you please add a test for this?

[matez0]

This expression is equivalent (see operator precedence) with

(fields and isinstance(fields[0][1], float) and fields[0][1] == 1.0) or fields[0][1] == -1.0

So, when fields is evaluated as False, then fields[0] will raise an IndexError (or other error, if it is not a list) and that may not be what we want.

Note: Even if and and or would have the same precedence, the same situation could be possible. That is why it is more readable to use parenthesis when different logical operators are present in an expression.

[gaurav25122000]

This is called short circuiting and when fields is evaluated as False, it won't go ahead to check the further conditions

[matez0]

The and sequence is not evaluated completely because of short circuiting, but the left side of or yes.
Try this out:

0 and 1 and 2 or 3

The expression can be parenthesis-ed as (0 and 1 and 2) or 3

This conditional expression introduce a new bug.

If you would write test for the case when bool(fields) is Ḟalse˙, it would crash probably with IndexError.

[matez0]

To help other potential contributors to understand the code and the change, it would be nice to give a context in the pull request description or a reference to the issue where the context is given.

About the commit message:

Added conversion for float to int for existing indexes while being fe…
…tched from mongo database due to pymongo check of indexes being 1,-1 or 2d

According to git conventions, the subject should be in imperative present tense.
The current subject is not a sentence and ambiguous (added is not a past tense verb in correct grammar).

Convert float indexes to int while being fetched from mongo database due to pymongo check of indexes being 1,-1 or 2d

This is already in imperative present tense, but for me, it is not clear what the part due to... means and it is too long.
The subject line should give a hint why the change was needed and the details part of the commit message should explain why in simple technical English. What the change is that is visible in the diff.

These could cost some effort for the contributor, but would save time for others and speed up the development of the project.

[gaurav25122000]

Hi @gaurav25122000 Thank you for the PR! Could you please add a test for this?

Can't find the test cases for this particular function

[roman-right]

Can't find the test cases for this particular function

This is true. Could you please add one? @gaurav25122000

[github-actions]

This PR is stale because it has been open 45 days with no activity.

[github-actions]

This PR was closed because it has been stalled for 14 days with no activity.

[97k]

I am facing this same issue.
is it going to make it in next release?

I moved a lot of code from my custom wrapper to beanie only to find out I am not able to use it because of this issue!

[97k]

I can add a test quick if @gaurav25122000 is not working on this one?

cc: @roman-right