This is an archive of the discontinued Mercurial Phabricator instance.

similar: add condition to avoid Zerodivisonerror in function _score() (issue6099)
Needs RevisionPublic

Authored by akshjain.jain74 on Mar 12 2019, 2:09 PM.

Download Raw Diff

Details

Reviewers

durin42
baymax

Group Reviewers

hg-reviewers

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

akshjain.jain74 created this revision.Mar 12 2019, 2:09 PM

Herald added a reviewer: durin42. · View Herald TranscriptMar 12 2019, 2:09 PM

Herald added a reviewer: hg-reviewers. · View Herald Transcript

Herald added a subscriber: mercurial-devel. · View Herald Transcript

pulkit added a subscriber: pulkit.Mar 13 2019, 4:54 AM

pulkit added inline comments.

hgext/histedit.py
1127 ↗	(On Diff #14477)	Is this change required for this patch?

Oh sorry ,my bad I think that is by mistake I will correct it now
Ithink while committing I added that file by mistake

I find it troubling that we now have contributors that don't follow #1 in https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist. Potentially #4 too (as Pulkit commented).

Seriously, the title is terrible at describing what this patch fixes and where. The commit message could briefly mention why/when this condition is even required.

mercurial/similar.py
66–67	This is some... interesting syntax. Is this valid Python?

av6 added inline comments.Mar 13 2019, 5:23 AM

mercurial/similar.py
66–68	This returns None in some cases, and code that uses `_score()` and `score()` tries to compare it to an integer. In Python3 `None > 1` raises TypeError.

Sorry for the various mistake
I will correct everything in an hour thanks

akshjain.jain74 marked an inline comment as done.Mar 13 2019, 1:29 PM

akshjain.jain74 retitled this revision from Experimental features: Add condition to for float number (issue6099) to ZeroDivisionError: Add condition to avoid Zerodivisonerror due to float number (issue6099).Mar 13 2019, 1:34 PM

akshjain.jain74 updated this revision to Diff 14482.

Okay, let's go over #1 in https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist once more. If you want to know what a good "topic" is, look at what other people do. How patches that get accepted generally look. How bug-fixing commits are worded.

Secondly, about that capitalization. I'm sorry if this is some sort of silly autocorrect that's doing it for you, I really hope you're not writing commit messages on a phone.

mercurial/similar.py
66–69	`None > 1` is still an issue, see my comment above.

sorry for that , but can you tell me in what condition this function will return none value?

actually i am not getting what can be the proper topic for this :(

akshjain.jain74 retitled this revision from ZeroDivisionError: Add condition to avoid Zerodivisonerror due to float number (issue6099) to similar: add condition to avoid Zerodivisonerror in function _score() (issue6099).Mar 13 2019, 4:17 PM

akshjain.jain74 updated this revision to Diff 14487.

akshjain.jain74 marked an inline comment as done.Mar 13 2019, 4:17 PM

Can you try to add a test for this?

mercurial/similar.py
66–68	we can do `if lengths:` here.
68	no need for this else. you can `return 0` without else.

In D6123#89410, @pulkit wrote:

Can you try to add a test for this?

yes i can definitely try to add test for this

thanks for the review

akshjain.jain74 marked 2 inline comments as done.Mar 15 2019, 10:19 AM

akshjain.jain74 updated this revision to Diff 14508.

akshjain.jain74 added inline comments.Mar 15 2019, 10:21 AM

mercurial/similar.py
68	yes actually 😅 i did'nt noticed that

Gentle ping on this. This has been around for a long time, what's the status of it?

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Apr 28 2020, 4:18 AM

Herald added a subscriber: mercurial-patches. · View Herald TranscriptApr 28 2020, 4:18 AM

Revision Contents
Changeset List

			Path	Packages
M			mercurial/similar.py (4 lines)

Commit	Parents	Author	Summary	Date
		Akshit Jain		Mar 12 2019, 1:58 PM

Diff 14508

mercurial/similar.py

	# count the number of bytes in each			# count the number of bytes in each
	equal = 0			equal = 0
	matches = mdiff.blocks(text, orig)			matches = mdiff.blocks(text, orig)
	for x1, x2, y1, y2 in matches:			for x1, x2, y1, y2 in matches:
	for line in lines[y1:y2]:			for line in lines[y1:y2]:
	equal += len(line)			equal += len(line)

	lengths = len(text) + len(orig)			lengths = len(text) + len(orig)
				if lengths:
	return equal * 2.0 / lengths			return equal * 2.0 / lengths
				av6Unsubmitted Done This is some... interesting syntax. Is this valid Python? av6: This is some... interesting syntax. Is this valid Python?
				return 0
				av6Unsubmitted Done This returns None in some cases, and code that uses `_score()` and `score()` tries to compare it to an integer. In Python3 `None > 1` raises TypeError. av6: This returns None in some cases, and code that uses `_score()` and `score()` tries to compare…
				pulkitUnsubmitted Done we can do `if lengths:` here. pulkit: we can do `if lengths:` here.
				pulkitUnsubmitted Done no need for this else. you can `return 0` without else. pulkit: no need for this else. you can `return 0` without else.
				akshjain.jain74AuthorUnsubmitted Not Done yes actually 😅 i did'nt noticed that akshjain.jain74: yes actually 😅 i did'nt noticed that

				av6Unsubmitted Not Done `None > 1` is still an issue, see my comment above. av6: `None > 1` is still an issue, see my comment above.
	def score(fctx1, fctx2):			def score(fctx1, fctx2):
	return _score(fctx1, _ctxdata(fctx2))			return _score(fctx1, _ctxdata(fctx2))

	def _findsimilarmatches(repo, added, removed, threshold):			def _findsimilarmatches(repo, added, removed, threshold):
	'''find potentially renamed files based on similar file content			'''find potentially renamed files based on similar file content

	Takes a list of new filectxs and a list of removed filectxs, and yields			Takes a list of new filectxs and a list of removed filectxs, and yields
	(before, after, score) tuples of partial matches.			(before, after, score) tuples of partial matches.

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	14477		Mar 12 2019, 2:09 PM	★	★
Diff 2	14482		Mar 13 2019, 1:34 PM	★	★
Diff 3	14487		Mar 13 2019, 4:17 PM	★	★
Diff 4	14508		Mar 15 2019, 10:19 AM	★	★