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

Stock Ageing TypeError: ‘<’ not supported between instances of ‘int’ … #22206

Merged
merged 4 commits into from Jun 12, 2020

Conversation

clarkejj
Copy link
Contributor

…and ‘str’

Re this user report https://discuss.erpnext.com/t/stock-ageing-report-typeerror-not-supported-between-instances-of-int-and-str/62605

Notes:

ASSUMPTION: This error arises in current version-12 production?

WARNING: This proposed change has not been tested

Please provide enough information so that others can review your pull request:

Explain the details for making this change. What existing problem does the pull request solve?

Screenshots/GIFs

@@ -180,14 +180,14 @@ def get_fifo_queue(filters, sle=None):
qty_to_pop = abs(d.actual_qty)
while qty_to_pop:
batch = fifo_queue[0] if fifo_queue else [0, None]
if 0 < batch[0] <= qty_to_pop:
if 0 < cint(batch[0]) <= qty_to_pop:
Copy link
Member

Choose a reason for hiding this comment

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

flt should be used instead of cint as qty can be fraction value as well

@deepeshgarg007 deepeshgarg007 self-assigned this Jun 11, 2020
As reviewer Deepesh Garg advised thanks!  batch[0] can be int of float - see line 49 - so change proposed cint to flt
Copy link
Contributor Author

@clarkejj clarkejj left a comment

Choose a reason for hiding this comment

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

As Deepesh advised on review of my original proposed commit - change cint to flt since batch[0] can be int or float - see line 49

Thanks Deepesh!

transferred_item_details[(d.voucher_no, d.name)].append(fifo_queue.pop(0))
else:
# all from current batch
batch[0] -= qty_to_pop
flt(batch[0]) -= qty_to_pop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
flt(batch[0]) -= qty_to_pop
batch[0] = flt(batch[0]) - qty_to_pop

Can't assign values to a function call so flt(batch[0]) is problematic

…obvious to me in retrospect, but a good relearning experience in any case
Copy link
Contributor Author

@clarkejj clarkejj left a comment

Choose a reason for hiding this comment

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

Many thanks for your patient pointer Marica - that assignment violation is clear to me in retrospect, but a good relearning and change commit exercise in any case!

@clarkejj
Copy link
Contributor Author

Hi Deepesh and Marica - so once this fix is ok with all checks passing, will you then push this to develop, version-12-hotfix and other backport expected branches where required?

@deepeshgarg007
Copy link
Member

deepeshgarg007 commented Jun 12, 2020

@Mergifyio backport version-12-hotfix

@deepeshgarg007 deepeshgarg007 merged commit bf66d1d into develop Jun 12, 2020
@clarkejj
Copy link
Contributor Author

Excellent thanks for the process learning coaching Deepesh and Marica

John

@marination
Copy link
Collaborator

@clarkejj Can you please send these changes on version-12-hotfix as well ?

clarkejj added a commit that referenced this pull request Jun 25, 2020
This PR combines these two PRs below applied to develop, to be applied to version-12-hotfix

(The mergifyio process failed for some reason.)

#22430

#22206
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

3 participants