merge versions and release new version about fpdf2 HOT 14 CLOSED

Awesome!
Great work 👍
I'll try to take a look at this on next Monday, to make sure all works fine & add missing tests if need be.
Do not hesitate to ping me back if you don't hear from me in next weeks ;)

Just a note: when I click on your link I end up on https://github.com/reingart/pyfpdf, but I guess you want the Pull Request to aim your repo ?

Thank you for working on making this lib better!

from fpdf2.

@Lucas-C yes, definitely meant to link to this instead https://github.com/alexanderankin/pyfpdf/compare/master...alexanderankin:merging_lucas-c?expand=1

from fpdf2.

I have found the code comments you left with the unfinished code merge, and I'm working on it.

It's really nice to have proper standards unit tests!

However I am quite sad that you introduced several dependencies to your fork: https://github.com/alexanderankin/pyfpdf/blob/master/setup.py#L35
Requiring to install a huge library like Numpy in order to use this lib is a bit of a deal-breaker for me...

It seems to be only used in get_img_info in fpdf/image_parsing.py so far. Would you please consider making it an optionnal dependency ?

from fpdf2.

I added a commit on merging_lucas-c with my work so far.

from fpdf2.

Latest update: I've got rid of numpy and the current version works fine with my project thats uses this lib (https://github.com/Lucas-C/undying-dusk) under Python 3.

What remains to be done for this branch to be merged:

• some bugs remain with Python 2 ( @alexanderankin do you really want to keep compatibility with Python 2 ??)
• some hash-based unit tests are broken:

  FAIL: test_insert_bmp (image.image_types.insert_images.InsertImagesTest)
  FAIL: test_insert_gif (image.image_types.insert_images.InsertImagesTest)
  FAIL: test_insert_jpg (image.image_types.insert_images.InsertImagesTest)
  FAIL: test_insert_png (image.image_types.insert_images.InsertImagesTest)
  FAIL: test_insert_png_files (image.png_images.png_file_test.InsertPNGSuiteFiles)

from fpdf2.

@Lucas-C No, I think at this point supporting python 2 is not strictly necessary, but that may motivate a major revision (which means i may have wasted 2, but perhaps that means 2.1 is more fitting?) I will try to take a look at the unit tests.

I don't remember what numpy was being used for, but i think dependencies are good (coming from the npmjs world) and i thought that adding a font library dependency would have been a next step since about 75% complaints are from font errors on non ascii chars. The alternative is returning to the pdf spec, which i spent a lot of time studying, but that was 2 years ago now.

Now that I'm reviewing it, i realize I've added a Pillow dependency and a numpy to deal with the same issue. Is it certain that all the stuff can be fixed with Pillow alone? I think that would be the preferable solution (if not finding an even higher level library suitable for pdf so as to further decrease the code in this library needs to handle pdf-specific use cases.

Looking at the broken tests, it looks like they are producing pdfs without images entirely. compare with the way those the behavior on master, at least the first one is visually different. I've attached the patch (.txt so github allows it) undelete.patch.txt which removes the os.unlink files which means after the tests run (you can run specific ones, i just forget how - feature of unittest i think, so should be documented somewhere), you can ask git to list new files and it shows you the outputs of the tests which weren't os.unlink'ed.

i've got a deadline on Friday. I can release 2.1.0rc1 sooner if youre cool with that, I just checked pep 440, seems it could be alpha, beta, release candidate. I dont want to publish a new version until i get the tests and images working, which should be this weekend. Also, any strong opinions on whether it should be 2.1 or 3?

diff --git a/test/image/image_types/insert_images.py b/test/image/image_types/insert_images.py
index a5c2e50..b78aaed 100644
--- a/test/image/image_types/insert_images.py
+++ b/test/image/image_types/insert_images.py
@@ -30,7 +30,7 @@ class InsertImagesTest(unittest.TestCase):
 
     test_hash = calculate_hash_of_file(test)
     self.assertEqual(test_hash, "98e21803d01d686504238cb17a636c32")
-    os.unlink(test)
+    #os.unlink(test)
 
   def test_insert_png(self):
     pdf = fpdf.FPDF()
@@ -45,7 +45,7 @@ class InsertImagesTest(unittest.TestCase):
 
     test_hash = calculate_hash_of_file(test)
     self.assertEqual(test_hash, "17c98e10f5c0d95ae17cd31b0f6a0919")
-    os.unlink(test)
+    #os.unlink(test)
 
   def test_insert_bmp(self):
     pdf = fpdf.FPDF()
@@ -60,7 +60,7 @@ class InsertImagesTest(unittest.TestCase):
 
     test_hash = calculate_hash_of_file(test)
     self.assertEqual(test_hash, "49e5800162c7b019ac25354ce4708e35")
-    os.unlink(test)
+    #os.unlink(test)
 
   def test_insert_gif(self):
     pdf = fpdf.FPDF()
@@ -75,7 +75,7 @@ class InsertImagesTest(unittest.TestCase):
 
     test_hash = calculate_hash_of_file(test)
     self.assertEqual(test_hash, "be9994a5fadccca9d316c39d302f6248")
-    os.unlink(test)
+    #os.unlink(test)
 
 if __name__ == '__main__':
   unittest.main()
diff --git a/test/image/png_images/png_file_test.py b/test/image/png_images/png_file_test.py
index d2f6f75..f1defd4 100644
--- a/test/image/png_images/png_file_test.py
+++ b/test/image/png_images/png_file_test.py
@@ -56,7 +56,7 @@ class InsertPNGSuiteFiles(unittest.TestCase):
     self.assertEqual(test_hash, "0085260bea512b9394ce1502b196240a")
 
     # self.assertEqual(test_hash, "4f65582566414202a12ed86134de10a7")
-    os.unlink(outfile)
+    #os.unlink(outfile)
 
 if __name__ == '__main__':
   # http://stackoverflow.com/questions/9502516/how-to-know-time-spent-on-each-

from fpdf2.

Thanks for the detailed answer!

• let's go with dropping support Python 2 then ! :)
  I've updated setup.py accordingly in the branch, and the unit tests executed by GitHub Actions will only ran under Python 3.6, 3.7, 3.8 & 3.9.

• I don't have anything against dependencies in general. I'm totally fine with the other 3 (Pillow, six & future).
  It's just that Numpy can be a pain to install : I'm mostly working on Windows, and have always encountered issues with it, either with Cygwin or WSL. Moreover, while getting rid of Numpy on this branch, I replaced ~40 lines of multi-dimensionnal arrays fiddling by a 10-lines easily testable & reusable function (source), so I think it's not a big loss in terms of code clarity & maintainability ;)
  I don't want to sound to categorical / pushy here, though 😅

• thank you for looking at the broken tests!
  If they are visually different, they need to be looked at for sure.
  By the way, I'm not a big fan of hash-based assertions, as it is hard to quickly understand the origin of the problem when they fail. However I don't know of any other approach for ensuring PDF are consistently produced... Maybe using a PDF-parsing lib like pikepdf to check that the PDFs generated by the unit tests have a the expected structure ? What do you think ?

• there is really no hurry here! Please focus on your deadlin for tomorrow 😉
  As far as I'm concerned, no need to make a release anytime soon neither.
  We can take the time to have everything ready for a v2.1.
  I think a v3 would maybe be too much... The changes brought by this branch are performance improvements mainly, not a full lib API change!

• would you be open to adopt a CHANGELOG.md file for this project?
  Like this one for example.
  I've used them in dozens of projects, and they are quite handy.

from fpdf2.

I fixed the remaing tests, all is green ! 🎉 🙌
-> https://github.com/alexanderankin/pyfpdf/actions/runs/372720846

from fpdf2.

ok there it is. https://pypi.org/project/fpdf2/2.1.0rc1/

right now pip install fpdf2==2.1.0rc1 is available while pip install fpdf2 gets 2.0.6. Will release 2.1.0 in the next couple of days hopefully. This is basically me executing the motions but giving myself time to find the mistakes.

from fpdf2.

Great!

When the branch is merged in master I'll submit a PR to a CHANGELOG.md if that is ok for you.

from fpdf2.

Hi!
Could you please make a new release on Pypi?
I think this issue could be closed then :)

from fpdf2.

I'll wait for this also

from fpdf2.

@Lucas-C @hasii2011 done! https://pypi.org/project/fpdf2/#history

from fpdf2.

Thank you!

from fpdf2.