[FIX] MyAnimeList - Use JSON instead of XML by BrutuZ · Pull Request #2183 · Flexget/Flexget

BrutuZ · 2018-07-31T20:26:32Z

Fixed MyAnimeList input plugin by switching from PHP\XML to JSON parsing.
ANIME_TYPE item 'series' updated to 'tv' in order to better reflect how the site refers to these.

Motivation for changes: Plugin stopped working after site maintenance and backend changes

Detailed changes:

Rewrote most of the segment responsible for fetching the data to use the JSON address and handle its data for Flexget entries.

Addressed issues:

Fixes my_anime_list plugin is no longer functional #2152 .

Implemented feature requests:

None

Config usage if relevant (new plugin or updated schema):

type: series should be updated to type: tv

To Do:

Review
Pull

Fixed MyAnimeList input plugin by switching from PHP\XML to JSON parsing. ANIME_TYPE item 'series' updated to 'tv' in order to better reflect how the site refers to these.

liiight · 2018-07-31T22:01:55Z

flexget/plugins/input/my_anime_list.py

        if not isinstance(selected_types, list):
            selected_types = [selected_types]

+        for i in range(len(selected_status)):


Looks like you can use enumerate here

Fixed that and the other comments below. Should open a new PR or can a commit be added to this one?

liiight · 2018-07-31T22:02:41Z

flexget/plugins/input/my_anime_list.py

+
        try:
-            list_response = task.requests.get('https://myanimelist.net/malappinfo.php', params=parameters)
+            list.response = task.requests.get('https://myanimelist.net/animelist/' + config['username'] + '/load.json')


The dot is probably a typo, right?

Right, my bad.

liiight · 2018-07-31T22:03:46Z

flexget/plugins/input/my_anime_list.py

-                if entry.isvalid():
-                    entries.append(entry)
-
+            js = json.loads(list.response.text.encode('utf-8'))


No need for this, just call .json() on the response

That would also save the import json line at the top

liiight · 2018-07-31T22:04:29Z

flexget/plugins/input/my_anime_list.py

+            raise plugin.PluginError('Error reading JSON')
+
+        for anime in js:
+            if (anime["status"] in selected_status


Funky indentation

Broke it down to comply with styling recommendation (up to 120 characters line) and make the conditions easier to read

In that case, something like this is probably better:

has_selected_status = anime["status"] in selected_status or config['status'] == 'all' has_selected_type = anime["anime_media_type_string"].lower() in selected_types or config['type'] == 'all' if has_selected_status and has_selected_type:

Lines are still <120 characters, and it's significantly easier to understand.

Fixed MyAnimeList input plugin by switching from PHP\XML to JSON parsing. ANIME_TYPE item 'series' updated to 'tv' in order to better reflect how the site refers to these.

liiight · 2018-08-01T03:49:25Z

flexget/plugins/input/my_anime_list.py

-                    entries.append(entry)
-
+            js = list_response.json()
+        except:


This is too broad. I imagine you want to catch requests.HTTPError here

The HTTP request is done earlier, this just decodes the response.

What exception is raised on decode errors seems to depend on the underlying JSON library used by requests, and possibles exceptions include at least simplejson.errors.JSONDecodeError and json.decoder.JSONDecodeError.

At the very least, the statement should be updated to except Exception.

liiight · 2018-08-01T03:50:32Z

flexget/plugins/input/my_anime_list.py

        if not isinstance(selected_types, list):
            selected_types = [selected_types]

+        for i,j in enumerate(selected_status):


Missing whitespace after comma. Also, if possible, please provide meaningful variable names.

tobbez · 2018-08-01T03:50:42Z

flexget/plugins/input/my_anime_list.py

+    'on_hold' : 3,
+    'dropped' : 4,
+    'plan_to_watch' : 6,
+    'all' : 7


Dictionary keys and : should not be separated by whitespace.

tobbez · 2018-08-01T03:52:46Z

flexget/plugins/input/my_anime_list.py

-        selected_status = config.get('status', list(STATUS.values()))
-        selected_types = config.get('type', list(ANIME_TYPE.values()))
+        selected_status = config.get('status', list(STATUS.keys()))
+        selected_types = config.get('type', list(ANIME_TYPE))


Assuming the defaults set in the schema ensure these have values, the second parameter to .get will never be used, and these could be replaced by config['status']/config['type'].

tobbez · 2018-08-01T03:54:56Z

flexget/plugins/input/my_anime_list.py

            selected_types = [selected_types]

+        for i,j in enumerate(selected_status):
+            selected_status[i] = STATUS[j]


Replacing these two lines with selected_status = [STATUS[s] for s in selected_status] would be more idiomatic.

tobbez · 2018-08-01T04:02:41Z

flexget/plugins/input/my_anime_list.py

+            raise plugin.PluginError('Error reading JSON')
+
+        for anime in js:
+            if (anime["status"] in selected_status


In that case, something like this is probably better:

has_selected_status = anime["status"] in selected_status or config['status'] == 'all' has_selected_type = anime["anime_media_type_string"].lower() in selected_types or config['type'] == 'all' if has_selected_status and has_selected_type:

Lines are still <120 characters, and it's significantly easier to understand.

tobbez · 2018-08-01T04:06:06Z

flexget/plugins/input/my_anime_list.py

+                    url = "https://myanimelist.net" + anime["anime_url"],
+                    mal_name = anime["anime_title"],
+                    mal_poster = anime["anime_image_path"],
+                    mal_type = anime["anime_media_type_string"]


Equals sign (=) between parameter names and values should not be surrounded by white space.

tobbez · 2018-08-01T04:12:08Z

flexget/plugins/input/my_anime_list.py

+
        try:
-            list_response = task.requests.get('https://myanimelist.net/malappinfo.php', params=parameters)
+            list_response = task.requests.get('https://myanimelist.net/animelist/' + config['username'] + '/load.json')


config['username'] may require URL encoding (depending on what characters are allowed in usernames).

tobbez · 2018-08-01T04:12:34Z

flexget/plugins/input/my_anime_list.py

+                    mal_poster = anime["anime_image_path"],
+                    mal_type = anime["anime_media_type_string"]
+                )
+                entries.append(entry)


The Entry could be constructed directly when appended. No need to assign it to an intermediate variable first, i.e:

entries.append(Entry( title=anime["anime_title"], ... ))

Nothing wrong with a little brevity there, assigning it first is fine

tobbez · 2018-08-01T17:41:13Z

flexget/plugins/input/my_anime_list.py

            selected_types = [selected_types]

+        for index, value in enumerate(selected_status):
+            selected_status[index] = STATUS[value]


selected_status = [STATUS[s] for s in selected_status] would still be more idiomatic than these two lines.

Other than that: LGTM.

[FIX] Use JSON instead of XML for MyAnimeList

6101cf7

Fixed MyAnimeList input plugin by switching from PHP\XML to JSON parsing. ANIME_TYPE item 'series' updated to 'tv' in order to better reflect how the site refers to these.

liiight reviewed Jul 31, 2018

View reviewed changes

BrutuZ added 3 commits July 31, 2018 20:12

[Fixed] MyAnimeList - Use JSON instead of XML

e06bce1

Fixed MyAnimeList input plugin by switching from PHP\XML to JSON parsing. ANIME_TYPE item 'series' updated to 'tv' in order to better reflect how the site refers to these.

Corrected IF indentation

06a6ff8

JSON conversion

a3a2531

liiight reviewed Aug 1, 2018

View reviewed changes

tobbez reviewed Aug 1, 2018

View reviewed changes

Updates based on feedback

8ba2789

liiight approved these changes Aug 1, 2018

View reviewed changes

tobbez reviewed Aug 1, 2018

View reviewed changes

Idiomatic loop

8205104

tobbez approved these changes Aug 2, 2018

View reviewed changes

liiight merged commit 9051386 into Flexget:develop Aug 2, 2018

BrutuZ deleted the patch-1 branch May 21, 2019 17:55

Conversation

BrutuZ commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for changes: Plugin stopped working after site maintenance and backend changes

Detailed changes:

Addressed issues:

Implemented feature requests:

Config usage if relevant (new plugin or updated schema):

To Do:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobbez Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tobbez Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BrutuZ commented Jul 31, 2018 •

edited

Loading

tobbez Aug 1, 2018 •

edited

Loading

tobbez Aug 1, 2018 •

edited

Loading