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

Adapt the code so to make it compatible with the ac/improve-code-quality branch of the object detector #8

Merged
merged 4 commits into from
Aug 30, 2023

Conversation

acerioni
Copy link
Member

A one-line update was enough to make the code compatible with the ac/improve-code-quality branch of the object detector => tile IDs must comply to the following format: (, , ), with leading and trailing round brackets.

@acerioni acerioni requested a review from cleherny July 28, 2023 08:36
@cleherny
Copy link
Collaborator

cleherny commented Aug 7, 2023

The changes you've made look good to me and the script works fine.
However, I noticed a problem with Python dependencies. Usually, I am performing the whole workflow with the requirements located in the proj-dqry repo. But, with the latest changes to the object-detector, I had a problem running scripts train.py and make_prediction.py. I get this error:
AttributeError: module 'PIL.Image' has no attribute 'LINEAR'
So I had to switch to object-detector requirements.txt to run those scripts.

The way I managed to make it run with the requirements located in proj-dqry is to specify the same version of pillow==9.5.0 and geopandas==0.11.1 as in the requirements.txt of the object-detector on github (branch ac/test-new-objdet-version-bis). The latest version of pillow==10.0.0 led to the above error. Maybe there is more investigation to perform to make sure the scripts are compatible with the most recent version of pillow, otherwise, we have to specify the working version in requirements.in.

@acerioni
Copy link
Member Author

acerioni commented Aug 7, 2023

The main question is: does the proj-dqry code base have, on its own, dependencies other than those of the object detector?

Duplicated tiles are detected on the basis of the `title` attribute instead of the geometry
@acerioni
Copy link
Member Author

acerioni commented Aug 7, 2023

Apparently, morecantile is the only dependency of the proj-dqry code base which is not shared with the object detector.

@acerioni
Copy link
Member Author

acerioni commented Aug 7, 2023

I just pushed a new commit, in which the requirements.in file takes advantage of the packaging of the object detector, introduced on last Friday (see https://github.com/swiss-territorial-data-lab/object-detector/tree/ac/add-setuptools).

Once that a 1st tag of the object detector will be created, the requirements.in will have to be updated in order to refer to the tag instead of the ac/add-setuptools branch.

@cleherny
Copy link
Collaborator

cleherny commented Aug 7, 2023

The new requirements.in looks great! It will be very useful.
Still have an issue, when installing the requirements.txt I get an error that I don't quite know how to solve:
requirements_error
Do I miss a step?
All the scripts are running fine except train_model.py:
AttributeError: module 'numpy' has no attribute 'bool'. np.boolwas a deprecated alias for the builtinbool. To avoid this error in existing code, use boolby itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, usenp.bool_ here.

In addition, I have done a test by replacing 'geometry' with 'title' in drop_duplicates and it takes about the same time (about 58s) to run for the prd workflow.

@acerioni
Copy link
Member Author

I had used latest as version number, which is not compliant with PEP 440 (see https://peps.python.org/pep-0440/). I've just pushed a new commit, in which the version number is 1.0rc1.dev1 (see https://github.com/swiss-territorial-data-lab/object-detector/blob/e3fe620b7a466854c7c0320b0053eb6aa5e79e4f/setup.py#L8).

@cleherny could you please repeat the test? Thanks!

@acerioni
Copy link
Member Author

Concerning the drop_duplicates, on my side the prepare_data.py with the prd configuration takes

  • 1.59 s if using title;
  • 17.97 s if using geometry.

@cleherny
Copy link
Collaborator

cleherny commented Aug 29, 2023

  • proj-dqry is running fine. I obtained similar results to the ones before the changes on the objdet.
  • I obtained similar running duration than your tests concerning drop-duplicates in prepare-data.py.
  • I have experienced an issue concerning the setup.py file. After installing dependencies, when I execute the command stdl-objdet I obtain the following error:
    image

@acerioni
Copy link
Member Author

The error was due to the fact that no argument was passed after stdl-objdet. Thanks to this commit the stdl-objdet script displays the help in case of missing arguments.

@cleherny cleherny merged commit ea1d098 into ch/dev_tm Aug 30, 2023
0 of 2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants