Add get data directories function #375

bryanwweber · 2016-10-26T04:53:24Z

Add a function to get the data directories that Cantera searches. Since we're using Boost in another spot now as a requirement, I thought it'd be OK to use that here as well. If not, we just need to implement our own string imploder to convert from std::vector<string> to a std::string.

speth · 2016-10-26T19:45:24Z

Yes, using Boost is definitely fine (and I was actually planning on starting to use the string algorithms to replace some of the functions defined in stringUtils.h). I also agree that having a function to make this information available would be useful.

The question I have is whether this function should return a single string or whether it should just return a vector<string>. When would you want the list of directories in this format, especially given the OS-dependence of the path separator?

bryanwweber · 2016-10-26T20:52:19Z

I chose that format, and the separator, assuming that most people would interact with the result as though it were something like the system PATH. I also wasn't sure how to translate the vector<string> into something Cython/the C library (for Matlab) would understand. Any pointers on that would be appreciated!

speth · 2016-10-26T21:14:12Z

I think Cython should understand how to translate vector<string>, it takes just a bit of fiddling to convert the strings to the correct type. For example, we already manage something similar with the Composition type which is std::map<string,double>.

For C or Matlab, returning a single string may indeed be an easier solution.

speth · 2016-10-27T21:55:37Z

Another option: what about having getDataDirectories take the separator as an argument?

bryanwweber · 2016-10-29T16:09:57Z

OK I added the argument and rebased onto master. I think returning the string makes the most sense, for easy interoperability with Clib and Matlab.

speth · 2016-10-29T17:35:03Z

interfaces/cython/cantera/utils.pyx

@@ -31,6 +31,10 @@ def add_directory(directory):
    """ Add a directory to search for Cantera data files. """
    CxxAddDirectory(stringify(directory))

+def get_data_directories(sep=':'):


I think you need to use os.pathsep instead of ':' here, otherwise this will fail badly on Windows.

It shouldn't matter, since the C++ returns just a string, and that's split into a list in Python, right?

On Windows, if you call getDataDirectories with : as the separator, it will return a string like:

C:\src\anaconda\envs\py33\lib\site-packages\cantera\data:C:\Program Files\Cantera\data

Which Python will then split as:

['C', '\\src\\anaconda\\envs\\py33\\lib\\site-packages\\cantera\\data', 'C', '\\Program Files\\Cantera\\data']

And actually, given that the Python method returns a list, it shouldn't take the separator as an argument, it should just use os.pathsep directly.

Right, I forgot about the drive letter... Thanks for the note

speth · 2016-10-29T17:37:16Z

interfaces/matlab/getdatadirectories.m

+function getdatadirectories(sep)
+% GETDATADIRECTORIES  Get a string of the directories searched for data files.
+% getdatadirectories(sep)
+% Get a string of the directores Cantera searches for data files, with each


misspelled 'directories'

speth · 2016-10-29T17:38:11Z

interfaces/matlab/getdatadirectories.m

@@ -0,0 +1,11 @@
+function getdatadirectories(sep)


Don't we camelCase most of the most of the Matlab function names?

speth · 2016-10-29T17:40:56Z

src/clib/ct.cpp

@@ -1420,6 +1420,17 @@ extern "C" {
        }
    }

+    int getDataDirectories(int buflen, char* buf, const char* sep)


I think the function should be named ct_getDataDirectories (some of the existing functions need to be updated to follow this convention). Also, I think you forgot to add ct.h.

speth · 2016-10-29T17:42:59Z

src/clib/ct.cpp

+        try {
+            string d = getDataDirectories(sep);
+            copyString(d, buf, buflen);
+            return int(d.size());


copyString was recently modified so that you can simply write:

return copyString(getDataDirectories(sep), buf, buflen);

speth · 2016-10-29T17:44:50Z

src/matlab/ctfunctions.cpp

+        // get string of data directories
+    case 5:
+        sep = getString(prhs[2]);
+        buflen = getDataDirectories(buflen, output_buf, sep);


I think it is more clear to just put 0 in place of buflen and output_buf in this initial call.

speth · 2016-10-29T21:45:40Z

doc/SConscript

@@ -212,7 +212,7 @@ if localenv['sphinx_docs']:
        '@Data': ['Air.m', 'constants.m', 'gasconstant.m', 'GRI30.m',
             'Hydrogen.m', 'Methane.m', 'Nitrogen.m', 'oneatm.m',
             'Oxygen.m', 'Water.m'],
-        '@Utilities': ['adddir.m', 'ck2cti.m', 'cleanup.m', 'geterr.m',]
+        '@Utilities': ['adddir.m', 'ck2cti.m', 'cleanup.m', 'geterr.m', 'getdatadirectories.m']


Need to update case here as well.

speth · 2016-10-30T21:33:24Z

I squashed a few of the revisions in preparation for merging this, and fixed two bugs:

for the Python version, os.path.sep is not the same as os.pathsep (on Linux, the former is '/' and the latter is ':')
In ctfunctions.cpp, the variable sep had not been declared, and the method called was incorrect (I guess you aren't currently compiling the Matlab toolbox)

However, I also noticed that the Matlab function does not work -- it currently does not return anything.

bryanwweber · 2016-10-30T22:41:46Z

Hmmm... I didn't get a chance to test the Matlab toolbox, I can do that when I get back to the office tomorrow. Thanks for fixing those bugs!

bryanwweber · 2016-10-31T15:04:03Z

OK I rebased this onto b01f9cd and fixed the Matlab bug. It returns a cell array of strings now, similar to the list in Python.

bryanwweber mentioned this pull request Oct 26, 2016

Make cython tests portable #376

Closed

bryanwweber force-pushed the add_get_data_dirs branch from 333e553 to ffd9f4c Compare October 29, 2016 16:08

speth requested changes Oct 29, 2016

View reviewed changes

speth reviewed Oct 29, 2016

View reviewed changes

speth force-pushed the add_get_data_dirs branch from 157ebaa to 75821c9 Compare October 30, 2016 21:19

speth approved these changes Oct 30, 2016

View reviewed changes

speth force-pushed the add_get_data_dirs branch from 75821c9 to 9b9f578 Compare October 30, 2016 21:26

bryanwweber added 6 commits October 31, 2016 10:10

Add function to get the data directories Cantera searches

2d3a553

Add C-Lib and Matlab interface to getDataDirectories

b8ed02a

Fix typo in docs for findInputFile

4532922

Fix rcond unintialized warning

e201ff5

Fix typo in application.h

bf9092d

Add config.h.build to gitignore

7071bf6

bryanwweber force-pushed the add_get_data_dirs branch from 9b9f578 to 7071bf6 Compare October 31, 2016 15:03

speth merged commit d59cae1 into Cantera:master Oct 31, 2016

bryanwweber deleted the add_get_data_dirs branch November 11, 2016 22:46

speth mentioned this pull request Nov 29, 2016

Updates for 2.3.0 beta #399

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add get data directories function #375

Add get data directories function #375

bryanwweber commented Oct 26, 2016

speth commented Oct 26, 2016

bryanwweber commented Oct 26, 2016

speth commented Oct 26, 2016

speth commented Oct 27, 2016

bryanwweber commented Oct 29, 2016

speth Oct 29, 2016

bryanwweber Oct 29, 2016 •

edited

Loading

speth Oct 29, 2016 •

edited

Loading

speth Oct 29, 2016

bryanwweber Oct 29, 2016

speth Oct 29, 2016

speth Oct 29, 2016

speth Oct 29, 2016

speth Oct 29, 2016

speth Oct 29, 2016

speth Oct 29, 2016

speth commented Oct 30, 2016

bryanwweber commented Oct 30, 2016

bryanwweber commented Oct 31, 2016

Add get data directories function #375

Add get data directories function #375

Conversation

bryanwweber commented Oct 26, 2016

speth commented Oct 26, 2016

bryanwweber commented Oct 26, 2016

speth commented Oct 26, 2016

speth commented Oct 27, 2016

bryanwweber commented Oct 29, 2016

Choose a reason for hiding this comment

bryanwweber Oct 29, 2016 • edited Loading

Choose a reason for hiding this comment

speth Oct 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

speth commented Oct 30, 2016

bryanwweber commented Oct 30, 2016

bryanwweber commented Oct 31, 2016

bryanwweber Oct 29, 2016 •

edited

Loading

speth Oct 29, 2016 •

edited

Loading