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

Latest code not working (undefined variables, wrong function call) #7

Open
stweil opened this issue Sep 7, 2018 · 8 comments
Open

Comments

@stweil
Copy link
Contributor

stweil commented Sep 7, 2018

The code uses undefined variables mets_doc and manifest_ident and calls _fill_manifest_metadata with only one instead of two arguments.

I had to fix those lines to get something which works partially:

diff --git a/demetsiiify/iiif.py b/demetsiiify/iiif.py
index 7b861db..1aeafc5 100644
--- a/demetsiiify/iiif.py
+++ b/demetsiiify/iiif.py
@@ -175,7 +175,7 @@ def _make_empty_manifest(ident, label):
         current_app.config['SERVER_NAME']))
     manifest_factory.set_iiif_image_info('2.0', 0)
     manifest = manifest_factory.manifest(ident=manifest_ident,
-                                         label=make_label(mets_doc.metadata))
+                                         label=label)
     return manifest
 
 
@@ -212,9 +212,12 @@ def make_manifest(ident, mets_doc, physical_map, thumbs_map):
     :returns:               Generated IIIF manifest
     :rtype:                 dict
     """
+    manifest_ident = '{}://{}/iiif/{}/manifest'.format(
+        current_app.config['PREFERRED_URL_SCHEME'],
+        current_app.config['SERVER_NAME'], ident)
     manifest = _make_empty_manifest(ident=manifest_ident,
                                     label=make_label(mets_doc.metadata))
-    _fill_manifest_metadata(manifest)
+    _fill_manifest_metadata(manifest, mets_doc.metadata)
 
     phys_to_canvas = {}
     seq = manifest.sequence(ident='default')
@jbaiter
Copy link
Owner

jbaiter commented Sep 7, 2018

Hey Stefan, thank you for pointing that out. I'm away this weekend, but will give the code a thorough look next week, including #8 and any other issues you might find :-)

@stweil
Copy link
Contributor Author

stweil commented Sep 10, 2018

This patch works better:

diff --git a/demetsiiify/iiif.py b/demetsiiify/iiif.py
index 7b861db..63dc9f6 100644
--- a/demetsiiify/iiif.py
+++ b/demetsiiify/iiif.py
@@ -174,8 +174,7 @@ def _make_empty_manifest(ident, label):
         current_app.config['PREFERRED_URL_SCHEME'],
         current_app.config['SERVER_NAME']))
     manifest_factory.set_iiif_image_info('2.0', 0)
-    manifest = manifest_factory.manifest(ident=manifest_ident,
-                                         label=make_label(mets_doc.metadata))
+    manifest = manifest_factory.manifest(ident=manifest_ident, label=label)
     return manifest
 
 
@@ -212,9 +211,9 @@ def make_manifest(ident, mets_doc, physical_map, thumbs_map):
     :returns:               Generated IIIF manifest
     :rtype:                 dict
     """
-    manifest = _make_empty_manifest(ident=manifest_ident,
+    manifest = _make_empty_manifest(ident=ident,
                                     label=make_label(mets_doc.metadata))
-    _fill_manifest_metadata(manifest)
+    _fill_manifest_metadata(manifest, mets_doc.metadata)
 
     phys_to_canvas = {}
     seq = manifest.sequence(ident='default')

@jbaiter
Copy link
Owner

jbaiter commented Sep 11, 2018

I'm currently refactoring the code quite a bit, so I can finally add some unit tests. In its current state, the whole logic is horribly coupled and very convoluted in some places, it's one of those "what was I thinking??" situations :-) expect an update by the weekend.

@stweil
Copy link
Contributor Author

stweil commented Sep 11, 2018

Great! I promise to give it a try then.

@stweil
Copy link
Contributor Author

stweil commented Sep 20, 2018

Thank you for pushing the refactored code. It still does not work for me.

Error: class uri 'gevent' invalid or not found:
[...]
ModuleNotFoundError: No module named 'greenlet'

@jbaiter
Copy link
Owner

jbaiter commented Sep 20, 2018

Did you try the Docker-based setup as per the instructions in the README?
Should be working now, seems like you need to explicitely add the greenlet dependency 🤔

@stweil
Copy link
Contributor Author

stweil commented Sep 21, 2018

Thank you. That's fixed now. This issue remains for the Docker based setup:

$ docker-compose run webapp python manage.py create
Traceback (most recent call last):
  File "manage.py", line 1, in <module>
    from flask import current_app
ModuleNotFoundError: No module named 'flask'

@jbaiter
Copy link
Owner

jbaiter commented Sep 21, 2018

Dang, forgot to fix the README 🙄 It should be:

$ docker-compose run webapp pipenv run manage create

I introduced pipenv with the refactor, which takes care of installing all the dependencies in a virtualen environment and automatically load that when executing the program.

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

No branches or pull requests

2 participants