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

Sample data is all over the place #1362

Closed
remram44 opened this issue Jun 3, 2015 · 10 comments
Closed

Sample data is all over the place #1362

remram44 opened this issue Jun 3, 2015 · 10 comments
Labels

Comments

@remram44
Copy link
Contributor

@remram44 remram44 commented Jun 3, 2015

On Linux, sample_data is in ${CMAKE_INSTALL_PREFIX}/sample_data; this is fine.

On Mac OS, sample_data ends up in ${CMAKE_INSTALL_PREFIX}/Library/Frameworks/Python.framework/Versions/Current/sample_data.

Anywhere, the data for uvcmetrics ends up in ${HOME}/uvcmetrics_test_data; this is inconvenient and wrong.

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Jun 3, 2015

@remram44 you can control where it goes with via the cmake option: UVCMETRICS_TEST_DATA_DIRECTORY

These are not uvcdat sample_data so actually your proposed location is just as wrong.

We cannot put them under the build directory has they need to be persistent.

A good/better location for this data is probably under share/metrics or something similar.

@remram44
Copy link
Contributor Author

@remram44 remram44 commented Jun 3, 2015

I did not propose a location, just pointing out that sample_data location on Mac is also wrong. That is a second problem.

@remram44
Copy link
Contributor Author

@remram44 remram44 commented Jun 3, 2015

${CMAKE_INSTALL_PREFIX}/share/{uvcdat,uvcmetrics} isn't the right location?

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Jun 3, 2015

@remram44 I don't really want to stick it inside the distribution because it's rather large. And you don't really want to download this data with every install. I got about 8 different version of uvcdat on my system at any given time. I could see it go under ${home}/.uvcdat though now that I think about it.

@doutriaux1
Copy link
Contributor

@doutriaux1 doutriaux1 commented Jun 3, 2015

now the thing about having it outside the distribution is that ONLY the one that builds get the data. So that's bad too... I guess under share/uvcmetrics is a good idea. I can always build with the option to alter the default path to my ${HOME} and not rebuild everytime. Let's go for ${CMAKE_INSTALL_PREFIX}/share/metrics
(since metrics is the actual name of the python package)

@allisonvacanti
Copy link
Contributor

@allisonvacanti allisonvacanti commented Jun 4, 2015

+1 for moving to ${CMAKE_INSTALL_PREFIX}/share/. UVCDAT adds quite a bit of clutter at the root install prefix, which would create quite a mess if someone was unfortunate enough to follow standard practices and set, say, /usr as their install prefix for a system installation.

Just to throw out another option, we could keep it out of the prefix entirely, and put the data in the uvcdat-testdata repo alongside the baselines. VTK does this (keeps sample inputs and baselines together) and it works rather well.

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jun 4, 2015

I would suggest that we follow this strategy:

  1. All of the data should get downloaded to ${CMAKE_INSTALL_PREFIX}/share
  2. Unless, user sets a specific path during the build time and then data goes there which will meet the requirements needed by developers to avoid multiple copy of data.

@remram44
Copy link
Contributor Author

@remram44 remram44 commented Jun 4, 2015

@aashish24 this is implemented by #1363, in the sense that you can still set UVCMETRICS_TEST_DATA_DIRECTORY. I don't think there ever was a variable to move the sample_data (for the record, it's 174MB).

@aashish24
Copy link
Contributor

@aashish24 aashish24 commented Jun 4, 2015

Thanks!, I/or others will have a look at it.

  • Aashish

@remram44
Copy link
Contributor Author

@remram44 remram44 commented Jun 22, 2015

Fixed by #1390

@remram44 remram44 closed this Jun 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants