Convert package to high-level Python wrapper around C++ extension

[a-hurst]

Closes #118, #43, #44, #63, and #72. This is a draft of my attempt at seamlessly wrapping the aggdraw C++ extension in a thin pure Python wrapper, which makes things like docstrings and generating/updating documentation vastly easier. Let me know what you think!

Docstrings have been converted/adapted from the original docs (now unreachable except via archive.org) into Google-style inline sphinxdoc, which should make generating and maintaining documentation much easier going forward. There are a number of areas where the docs could use some work (e.g. documenting all the different support colour strings aggdraw supports), but I figured further cleaning up the wording and clarity is a job for a later PR!

I had to reorganize how unit tests worked a bit in order for them to run with the new structure. This is because the selftest.py file defaults to using the aggdraw folder in the root of the PATH as the code to test (rather than the installed version), and then fails because the one in the source folder is missing the compiled C++ submodule (which gets added when actually installing to site-packages).

I still need to update the autodoc stuff to actually build using the new docstrings, but figured it was a good idea getting the basic approach up for review sooner than later! (EDIT: Now working, was less effort than I expected!)

[djhoese]

I converted this to a "Draft" PR given the [WIP] in the title. Let me know if that's not what you want and you're ready for a full review. I'll answer your comment in the issue in a minute.

[djhoese]

Nice! I had a couple questions inline.

[a-hurst]

Okay, so barring the possible switch to hatch for versioning I think the main thing preventing this PR from non-draft status is some more extensive testing. Specifically, the unit tests don't actually test a lot of the different Draw functions so there could be API breakage that we haven't noticed yet (e.g. the Font class and text functions). Do you have any other aggdraw testing code or aggdraw-based scripts you use locally for more extensive testing?

I put together a sort of test harness for my aggdraw-based drawing code to test for pixel-wise rendering differences between versions a little while ago but there's a lot of the aggdraw API I don't use. Maybe I could adapt into something to check for breakage here.

EDIT: Spoke too soon, looks like the Dib class might be broken. Unsure if that's something I did or if it's been broken for a while. Trying to set up an MSVC compiler on a Windows machine so I can test aggdraw on that and see what's going on.

[djhoese]

@a-hurst Any other changes you wanted to work on for this PR or ready for merging?

[djhoese]

Sorry, just saw your comment (browser didn't update the page). Reading...

[djhoese]

Don't spend too much time trying to debug Dib. Maybe we take this as a sign that it should be removed (maybe in a separate PR to make it clearer?)?

[djhoese]

...and no I don't have any real additional tests except for testing my own packages against it. I can do that tomorrow.

[a-hurst]

Don't spend too much time trying to debug Dib. Maybe we take this as a sign that it should be removed (maybe in a separate PR to make it clearer?)?

Think I've half-figured out the problem, at least enough to know it's not the wrapper's fault: the test itself runs fine, it's only when the test gets torn down at the end that it segfaults. I updated the test locally on my Windows test machine to use the aggdraw._aggdraw Dib class and that hard-crashes the exact same way. Guessing there's some sort of bug in the code responsible for freeing the object memory when torn down.

Anyway, the fact that there's a hard crash any time Python tries to free a Dib after use and that no one has reported this bug in the years it's likely existed means that it's probably fine to remove! I've also learned that Pillow has an ImageWin module that does basically the same thing as Dib, and since aggdraw can easily flush to Pillow surfaces anyway there's really no need for this to exist here anymore.

I agree that it should be removed in a later PR, though.

[a-hurst]

Okay, barring testing I think this is good to go! Gave it a once-over and fixed a few minor issues with the docs but everything else looked good. I've also update the list of issues closed by this in the top comment so they'll auto-close once merged.

[djhoese]

When trying to install locally, even though I have freetype installed, it doesn't seem to find it. I have some things I have to get some stuff done, but I'll try to debug this further tonight and merge this if a. it installs properly and b. the tests for my other package that uses aggdraw passes.

[djhoese]

Ok updating freetype from conda-forge fixed it. Must have been a bad version.

I do still see this in the build logs though:

  Checking if build backend supports build_editable ... done
  Running command Getting requirements to build editable
  <string>:29: SyntaxWarning: invalid escape sequence '\w'
  Trying freetype-config to find freetype library...

I'm not sure where that's coming from.

[djhoese]

Ah it is your regular expression. If I put an r in front of the string to make it a "raw" string then it stops complaining. Thoughts?

[a-hurst]

Ah it is your regular expression. If I put an r in front of the string to make it a "raw" string then it stops complaining. Thoughts?

Yep, I forgot that's the recommended practice. Thanks for fixing it!

I've also done some local tests and the core API and Font class seem to work as expected so I'm reasonably confident nothing major is still broken.

[djhoese]

Tested two different packages and all good. I think I'll merge this now and then we'll think about a release plan. I'm not sure if this is worthy of a 2.0. Unless maybe we completely remove Dib and include that. That would be good for 2.0 but so would forcing argument order (maybe a deprecation cycle in a pre-2.0 release).