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 method Spouse #275

Merged
merged 5 commits into from
Apr 5, 2022
Merged

Fixed method Spouse #275

merged 5 commits into from
Apr 5, 2022

Conversation

duck7000
Copy link
Contributor

Fixed this method, it took me about 2 weeks to cover all different kinds of data so i hope it will work in all cases.
Added test for this method.

@tboothman can you test this method and the test that i made?
It was very, very hard to get al data and separate it in the correct array elements. Re done this method 3 times and i hope it is in a working enough state

Spouse didn´t work at all, it should be fixed.
it took me 2 weeks to cover all the different kinds of data so i hope it will work in all cases.
@duck7000
Copy link
Contributor Author

duck7000 commented Mar 11, 2022

I must say that this method is also needlessly complicated.
Personally i would have get the whole spouse line as is and save it to an array element (will loose imdbid from the person if available)

@duck7000
Copy link
Contributor Author

@Thomasdouscha confirmed that this is working

Fixed bug when family part is not available on bio page and no spouse data either.
this leads to error Trying to get property 'nodeValue' of non-object
@tboothman
Copy link
Owner

tboothman commented Apr 5, 2022

Not looked at the code yet but the data it exposes is complicated, reducing it down to a string does not fulfil the point of this library which is extracting computer readable data from imdb.

Here's the underlying data for Miyazaki's spouse, which matches the fields that come out of this method so I think it's correct.
image

@tboothman tboothman merged commit 100932a into tboothman:master Apr 5, 2022
@tboothman
Copy link
Owner

Looks good 👍

@duck7000
Copy link
Contributor Author

duck7000 commented Apr 6, 2022

Thanks for merging, it was a pain in the back to make it work, lets hope that imdb don't change it..
And i removed as much reg exp as i could, that did make the code much more readable and it should be more robust to changes i think

@duck7000
Copy link
Contributor Author

duck7000 commented Apr 6, 2022

Well yes i agree with your argument that reducing to string is not the point of this library but that can make it difficult to maintain

@tboothman
Copy link
Owner

Thanks for merging, it was a pain in the back to make it work, lets hope that imdb don't change it..

Hah, yes. Well that's why when I noticed my very loose test of 'it returns something' started failing I opted to just ignore this method. Crazy complicated code

@duck7000
Copy link
Contributor Author

duck7000 commented Apr 6, 2022

Basically the code is complicated because imdb does not separate the data in html tags like span, p or even li. They just dumps it in a td. If i could find where that data is coming from would made my life a lot easier haha
so in order to split the data that was a massive undertaking, but i learned a whole lot more so that is worth something.

@tboothman
Copy link
Owner

tboothman commented Apr 6, 2022

Well ... imdb does have a graphql api that has a ton of data in it. https://api.graphql.imdb.com/
I was looking into it last time recommendations broke. You can get all (I think. Looks close) this spouse information from it.

query Person($id: ID!) {
  name(id: $id) {
    nameText {
      text
    }
    spouses {
      current
      attributes {
        text
        language {
          id
          text
        }
      }
      timeRange {
        fromDate {
          dateComponents {
            year
            month
            day
          }
        }
        toDate {
          dateComponents {
            year
            month
            day
          }
        }
      }
      spouse {
        name {
          id
          nameText {
		text
	 }
        }
      }
    }
  }
}

{
  "data": {
    "name": {
      "nameText": {
        "text": "Robin Williams"
      },
      "spouses": [
        {
          "current": false,
          "attributes": [
            {
              "text": "his death",
              "language": {
                "id": "en-US",
                "text": "English (United States)"
              }
            }
          ],
          "timeRange": {
            "fromDate": {
              "dateComponents": {
                "year": 2011,
                "month": 10,
                "day": 22
              }
            },
            "toDate": {
              "dateComponents": {
                "year": 2014,
                "month": 8,
                "day": 11
              }
            }
          },
          "spouse": {
            "name": {
              "id": "nm6699367",
              "nameText": {
                "text": "Susan Schneider"
              }
            }
          }
        },
        {
          "current": false,
          "attributes": [
            {
              "text": "divorced",
              "language": {
                "id": "en-US",
                "text": "English (United States)"
              }
            },
            {
              "text": "2 children",
              "language": {
                "id": "en-US",
                "text": "English (United States)"
              }
            }
          ],
          "timeRange": {
            "fromDate": {
              "dateComponents": {
                "year": 1989,
                "month": 4,
                "day": 30
              }
            },
            "toDate": {
              "dateComponents": {
                "year": 2010,
                "month": null,
                "day": null
              }
            }
          },
          "spouse": {
            "name": {
              "id": "nm0931265",
              "nameText": {
                "text": "Marsha Garces Williams"
              }
            }
          }
        },
        {
          "current": false,
          "attributes": [
            {
              "text": "divorced",
              "language": {
                "id": "en-US",
                "text": "English (United States)"
              }
            },
            {
              "text": "1 child",
              "language": {
                "id": "en-US",
                "text": "English (United States)"
              }
            }
          ],
          "timeRange": {
            "fromDate": {
              "dateComponents": {
                "year": 1978,
                "month": 6,
                "day": 4
              }
            },
            "toDate": {
              "dateComponents": {
                "year": 1988,
                "month": 12,
                "day": 6
              }
            }
          },
          "spouse": {
            "name": {
              "id": "nm0892239",
              "nameText": {
                "text": "Valerie Velardi"
              }
            }
          }
        }
      ]
    }
  }
}

@duck7000
Copy link
Contributor Author

duck7000 commented Apr 7, 2022

Looks interesting but i don't have a account so can't see/use it.
Is this api free available?

@duck7000 duck7000 deleted the spouse branch April 13, 2022 08:38
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.

2 participants