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

uri has been modified #740

Open
2 of 6 tasks
YaYaB opened this issue Jun 23, 2020 · 6 comments
Open
2 of 6 tasks

uri has been modified #740

YaYaB opened this issue Jun 23, 2020 · 6 comments

Comments

@YaYaB
Copy link
Contributor

YaYaB commented Jun 23, 2020

(A Markdown syntax reminder is available here: https://guides.github.com/features/mastering-markdown/)

Configuration

  • Version of DeepDetect:
    • Locally compiled on:
      • Ubuntu 16.04 LTS
      • Mac OSX
      • Other:
    • Docker
    • Amazon AMI
  • Commit (shown by the server when starting):
    c6d27f8

Your question / the problem you're facing:

I am using a training an image model with local paths to images. When I try to make the predictions on the lmdbs I get the uris of the images modified. A prefix is added before as an index, this is OK and expected. However the end of the uri has been cut.

After a look at the code I saw that in here, the uri's length is limited to 256. Unfortunately because of this, it is complicated to map the uri to the real image behind.

I see two possible solutions:

  • Indicate in the README that the URI can be cut if it is too long (max length 256 with index included)
  • Authorize a longer URI

I completely understand that the second option may not be desirable but it should be indicated somewhere that the URI might be modified.

@beniz
Copy link
Collaborator

beniz commented Jun 23, 2020

Ah hi @YaYaB good catch, as usual! So one way we have here is that we only store an id, or the relevant part of the URI, this is why we accomodate the 256 chars limit in our applications.

One of the reasons for using an internal ID to your application is that there can't be any good max size that'd fit for all cases, obviously.

Agreed that this should be part of the doc / readme, where do you believe it's most useful we add a warning ?

@YaYaB
Copy link
Contributor Author

YaYaB commented Jun 23, 2020

Being part of doc / readme would be amazing. And also having a warning when the lmdb is created could be nice as well

@beniz
Copy link
Collaborator

beniz commented Jun 23, 2020

OK, regarding the warning, we may allow it only once, because otherwise, it'd get printed millions of times for large datasets for which most of the time, the id in the db doesn't matter.

@YaYaB
Copy link
Contributor Author

YaYaB commented Jun 23, 2020

Yeah sure, once when the lmdb has finished or something like this

@beniz
Copy link
Collaborator

beniz commented Jul 3, 2020

See if #755 is fine for your need.

@YaYaB
Copy link
Contributor Author

YaYaB commented Jul 3, 2020

Yep it's perfect, it would be even better to have a note on the Readme or the api doc :)

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