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

fixed Sahih Muslim Introduction arabicnumbers #41

Open
wants to merge 4 commits into
base: 1
Choose a base branch
from

Conversation

unkn4wn
Copy link
Contributor

@unkn4wn unkn4wn commented Jan 7, 2023

I added the "arabicnumbers" and the book reference according to sunnah.com
I used the urdu translation as reference to determine where exactly to start in the introduction. I didnt change any "hadithnumber", so the translations should still be referenced correctly if they were correct before.
"arabicnumbers" should always be mentioned to have a consistent json structure and prevent errors, thats why the first "arabicnumber" is an empty string instead of removing the arabicnumber completely.
I already ran infofixer.

I hope there is a way to edit info.json and automatically edit all other related json files. Always changing everything manually after a minor change on info.json would be a pain.


This change is Reviewable

@fawazahmed0
Copy link
Owner

Thank you brother, but this PR might break things for others who have already implemented the api.
For example, you have used "arabicnumber": "Introduction 8" , but arabicnumber should always be a number and not string + number. I will look into this PR later

@unkn4wn
Copy link
Contributor Author

unkn4wn commented Jan 7, 2023

I don't think so, it would only break if you try to change hadithnumber to a string because its saved as a number.

Arabicnumber on the other hand is always saved as a String even if the string contains a number theoretically. For example here:
"hadithnumber": 1035,
"arabicnumber": "463.01"

So even if somebody retrieves the object "arabicnumber", he's already forced to obtain it as a string.

InshaaAllah!

@fawazahmed0
Copy link
Owner

well, they will parse that string as float or int, and if it's "Introduction 8" it will throw error.
Also "arabicnumber": "463.01" shouldn't have been a string, it should be a number(mistake done by me). I am not sure if I could change it now. (because people have already implemented this api).

@unkn4wn
Copy link
Contributor Author

unkn4wn commented Jan 10, 2023

I found some more wrong references. For example this one. It is also book 0 hadith 0 in info.json.
I wanted to fix the reference of this hadith too but it basically doesn't exist on sunnah.com. And it also doesn't exist on the Sahih Muslim Book from Darussalam

The text + hadithnumber of this hadith can only be found in your urd and ben translation. Maybe they used another arabic print ?

My suggestion would be to stick to "darussalam and sunnah.com" way of referencing and I will write down all ahadith which cant be found on "darussalam and sunnah.com", so we can batch delete these hadithnumbers later and order the hadithnumbers again.
But obv this is only my suggestion and I don't mind approaching it differently.

Thats how it looks like rn unedited:

{
"hadithnumber": 127,
"arabicnumber": "21.03",
"grades": [],
"reference": {
"book": 1,
"hadith": 35
}
},
{
"hadithnumber": 128,
"arabicnumber": 0,
"grades": [],
"reference": {
"book": 0,
"hadith": 0
}
},
{
"hadithnumber": 129,
"arabicnumber": "22",
"grades": [],
"reference": {
"book": 1,
"hadith": 36
}
},

@fawazahmed0
Copy link
Owner

fawazahmed0 commented Jan 10, 2023

I don't intend to delete any hadith ( I usually place the hadiths which does not have proper reference into section 0 )
Also incase of sahih muslim, you should compare arabicnumber with Reference given in sunnah.com.
The hadithnumber in sahih muslim is a sequence wise reference number (used by urdu etc translations) and this referencing is still used in many websites.

replaced "Introduction" with "0" to prevent parsing issues
@unkn4wn
Copy link
Contributor Author

unkn4wn commented Feb 7, 2023

Parsing issues should be solved. I replaced "Introduction 1/Introduction 2...." with "0.1/0.2...."

One issue remaining:
Hadithnumber 0 can be found inside the urdu json file but it isn't present inside the arabic json file.
It should also be present in all non urdu json files like this after adding it to info.json file:

{
			"hadithnumber": 0,
			"text": "",
			"grades": [],
			"reference": {
				"book": 0,
				"hadith": 0
			}
		},

@fawazahmed0
Copy link
Owner

Thank you brother, I will have to think if this will introduce any breaking changes. You need to wait a bit.
I will fix all the important issues (included this one) by end of ramadan inshallah.

@ahmedazhar05
Copy link
Contributor

well, they will parse that string as float or int, and if it's "Introduction 8" it will throw error. Also "arabicnumber": "463.01" shouldn't have been a string, it should be a number(mistake done by me). I am not sure if I could change it now. (because people have already implemented this api).

Why don't you fix all these major(code-breaking) bugs/issues in a new/major version-2?
That way, people who have already implemented this API will be able to continue using it without breaking their code and they can also move to the newer version if they like.

Ref.: Semantic versioning

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.

3 participants