qurator-spk / dinglehopper

An OCR evaluation tool
Apache License 2.0
58 stars 12 forks source link

Installing is broken in current master #77

Closed mikegerber closed 1 year ago

mikegerber commented 1 year ago

Due to confusion with pyenv etc. I am not sure and need to check again.

May be related to #76.

mikegerber commented 1 year ago

Yeah it's broken, and I didn't notice because it is

Great joy.

mikegerber commented 1 year ago

đź‘€ @bertsky

There was some confusion, I thought it was fixed, etc. etc.

bertsky commented 1 year ago

There was some confusion, I thought it was fixed, etc. etc.

Yes, still broken AFAIK. (For example, @kba just re-instated that version on ocrd_all, and voila it broke the CI again.

mikegerber commented 1 year ago

There was some confusion, I thought it was fixed, etc. etc.

Yes, still broken AFAIK. (For example, @kba just re-instated that version on ocrd_all, and voila it broke the CI again.

Yeah I know now (since Friday) and will fix it today

mikegerber commented 1 year ago

@kba I'm probably "over-communicating" but just to make sure: I thought the remaining issue is eynollah having the old style namespace but that's another, second problem.

kba commented 1 year ago

@kba I'm probably "over-communicating" but just to make sure: I thought the remaining issue is eynollah having the old style namespace but that's another, second problem.

Over-communicating is just fine :-) We should fix all the qurator namespace changes in one fell swoop, test them and create a new release of ocrd_all then. For now, the latest ocrd_all release contains dinglehopper in the commit just before the namespace change.

mikegerber commented 1 year ago

Note to self: In this case, testing the install in a fresh virtualenv also was necessary.

bertsky commented 1 year ago

We should fix all the qurator namespace changes in one fell swoop, test them and create a new release of ocrd_all then.

Is it really inevitable to change namespace pkg implementation? Is there no other way around the deprecation warning?

I am pretty sure this will fall on our feet again and again, because installations will not be equally "swooped" to the new versions likewise. (And we cannot even solve this via version requirements, because none of the qurator packages depend on each other.)

mikegerber commented 1 year ago

I have a fix ready (calling find_namespace_packages is necessary) but I can't push currently due to GitHub's problems.

kba commented 1 year ago

I am pretty sure this will fall on our feet again and again, because installations will not be equally "swooped" to the new versions likewise. (And we cannot even solve this via version requirements, because none of the qurator packages depend on each other.)

The decision is up to @qurator-spk/dev, I only noticed the deprecation warning. IIUC one of the reasonings for adapting this is that QURATOR project has ended, MMK project has begun and this would be part of the consolidation.

As for breaking installations: As long as people follow the upgrade instructions (deactivate/remove venv, make all for native; docker pull ocrd/all for docker) that should™️ not be a problem.

mikegerber commented 1 year ago

I've now removed the namespace prefix (s/qurator.dinglehopper/dinglehopper and associated changes) altogether.

mikegerber commented 1 year ago

Just to over-communicate again: It should be fixed now, and if you still have problems with git master, please open a new bug!