search

pytroll / aggdraw

Python package wrapping AGG2 drawing functionality
https://aggdraw.readthedocs.io/en/latest/
Other
99 stars 47 forks source link

Merge of pytroll/master with dov/agg24 #48

Closed dov closed 5 years ago

dov commented 5 years ago

I had to rewrite two tests to make them pass. I took care to manually verify that the new conditions make sense. To "prove" that the new path test is correct I created a small postscript "script" test-path-proof.ps that when viewed shows the correctness of the Bezier path I choose together with its flattened version.

Things still to fix:

djhoese commented 5 years ago

Thanks for this @dov. A couple questions:

  1. Could this PR be summarized as the following, right?: a. Move agg2 source directory to agg b. Update agg to 2.4 c. Update aggdraw source to match and work with agg 2.4 interfaces d. Update tests to work with new code
  2. The diff in this PR is making me wonder if the PKG-INFO file can be removed. Any idea if it is needed? It should be automatically created if I understand setuptools/distutils like I think I do.
  3. Any idea why the CHANGES file says the whole file was modified?

Other than that I'd like to update the README section you added (which I can do). I have no problem mentioning that you did the work for this, but it seems out of place now that it is being merged back in to the "main" repository (being at the top of the README).

dov commented 5 years ago
  1. Your summary pretty much nails it.
  2. I'm far from an expert on distutils, so do what you think is correct.
  3. Your version had DOS file endings which I removed. Do a git diff -w to see the "real changes"

Again, regarding the README file, do whatever you see fit. I more or less concatenated my README with your version, but probably didn't do a great job out of it.

Note that the agg version that I included contains some fixes that are not part of the "official" agg version (wherever that is these days).

djhoese commented 5 years ago

For 3, great!

For the agg changes, could you add those to the README in a new section near the bottom? "Changes to AGG" section title maybe? If you can link to specific commits for these changes that would be great. Github is only showing me the single merge commit right now for this PR which I suppose has to be because the two source distributions weren't shared. So if you can't link to commits that's ok too. A bulleted list summarizing the changes would make it easier for future users.

dov commented 5 years ago

Hmmm... I'm not sure exactly where I got the agg-2.4 sources from. They are very similar, but not identical to the agg svn tree at sourceforge. In my initial commit message from 2014 I did not give an indication where I got them from. It is probably a good idea to start from a clean slate and get he sources from sourceforge again. I'll replace my current agg sources with those sources, and will afterwards create a new pull request.

djhoese commented 5 years ago

This possibly: http://www.antigrain.com/download/index.html

dov commented 5 years ago

I compared those, and my sources are much closer to the source forge svn tree...

djhoese commented 5 years ago

Which one is newer?

dov commented 5 years ago

The svn version is newer. It has commits from 2018. In any case I managed to compiled against and verified that it passes all the tests. I'll create a new pull request.