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

[e621] extract extended metadata using e621-specific API endpoints #3425

Closed
wants to merge 11 commits into from

Conversation

ClosedPort22
Copy link
Contributor

@ClosedPort22 ClosedPort22 commented Dec 18, 2022

This is more of a discussion on the best way to implement features that are specific to e621. Considering that e621 "is a heavily modified fork of Danbooru", it may be necessary to separate e621 from the Danbooru extractors sooner or later.

Summary of the changes in this PR:

  • Extract notes using the /notes.json API endpoint
  • Extract pool metadata even when the subcategory is not "pool"
  • Treat e926 as a separate instance of e621. This allows the user to use different options for e621 and e926. However, the old config will no longer apply to e926.

While working on e621 I found that the Favorites extractor did not work for aibooru, so I've disabled it in supportedsites.py and left a TODO comment.

The /{post_id}.json endpoint doesn't return anything useful in the case
of e621.
This allows the user to configure e621 and e926 separately, but it is
not backward compatible since the old configuration will no longer
apply to e926.
- fix flake8 errors
- move e926 tests to e621.py
- remove 'Favorites' support from aibooru (because the code only works
  for e621)
@mikf
Copy link
Owner

mikf commented Jan 2, 2023

So you want to undo the "merge danbooru and e621 code" part from 563bd0e? I guess it is better that way, given the increasingly large differences.

I wouldn't inherit from DanbooruExtractor, though. Just do a clean split between the two sites and remove everything e621 specific from danbooru, like the FavoriteExtractor or this code here

file = post.get("file")
if file:
url = file["url"]
if not url:
md5 = file["md5"]
url = file["url"] = (
"https://static1.{}/data/{}/{}/{}.{}".format(
self.root[8:], md5[0:2], md5[2:4], md5, file["ext"]
))
post["filename"] = file["md5"]
post["extension"] = file["ext"]


If you want, I can also do this myself (in maybe a cleaner way) and you redo your metadata related changes after.

I would generally prefer not to include too many changes into one PR if possible. Also, git commit --amend makes it possible to "combine" commits and not have a new commit for every single change.

@ClosedPort22
Copy link
Contributor Author

I wouldn't inherit from DanbooruExtractor, though. Just do a clean split between the two sites and remove everything e621 specific from danbooru, like the FavoriteExtractor or this code here

Wouldn't that re-introduce some of the duplicate code, though? I wanted to be sure other Danbooru instances didn't break, since I didn't use them as often. I guess I missed some of the obvious e621-specific code...

If you want, I can also do this myself (in maybe a cleaner way) and you redo your metadata related changes after.

That would be fine, since this is mostly just a discussion. Thanks in advance.

I would generally prefer not to include too many changes into one PR if possible. Also, git commit --amend makes it possible to "combine" commits and not have a new commit for every single change.

I rewrite Git history a lot (including using git commit --amend) actually. I generally try to keep my commits small and follow the "atomic" commit principle. My apologies for the large pull requests (#3469 is even bigger than this). I'll make them smaller in the future.

mikf added a commit that referenced this pull request Feb 6, 2023
mikf added a commit that referenced this pull request Feb 6, 2023
@ClosedPort22 ClosedPort22 deleted the e621-extended-metadata branch February 6, 2023 15:10
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