File found by searching load path

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

File found by searching load path

Rik-4
There is a new bug report #55173 which which shows, possibly, an irrelevant
warning.

octave:1> load penny.mat
warning: load: '/usr/local/share/octave/5.0.0/data/penny.mat' found by
searching load path

I can see two approaches.  First, this warning has an ID,
Octave:data-file-in-path, and we could change the default on Octave startup
to disable this warning.  At that point, a user would have to actively turn
on this warning to see it.

Alternatively, I'm not sure why it is useful to know that the data file was
found in the load path.  We could just remove the code entirely.

Any votes one way or the other?

--Rik

Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

Przemek Klosowski-7
On 12/7/18 2:28 PM, Rik wrote:

> There is a new bug report #55173 which which shows, possibly, an irrelevant
> warning.
>
> octave:1> load penny.mat
> warning: load: '/usr/local/share/octave/5.0.0/data/penny.mat' found by
> searching load path
>
> I can see two approaches.  First, this warning has an ID,
> Octave:data-file-in-path, and we could change the default on Octave startup
> to disable this warning.  At that point, a user would have to actively turn
> on this warning to see it.
>
> Alternatively, I'm not sure why it is useful to know that the data file was
> found in the load path.  We could just remove the code entirely.
>
> Any votes one way or the other?

On the face of it, the warning is silly ("2+2 :  warning: addition:
result 4 found by applying the rules of arithmetic"). I think the intent
might have been to show the actual location of the file: I could see how
it might be useful to capture that, if 'load' allowed 2 return parameters :

[x,f]=load('penny.mat')

where x is the dataset being read and f is the filename.

All output should be catchable : it's neat that you can do

s=pkg('list')

for i=1:length(s)   printf("%s: %s\n",s{i}.name,s{i}.description)   end

I thought there were instances where this was not the case (I seem to
remember that pkg didn't always provide a way to capture the output),
but I can't recall where I ran into it.


BTW, is there a way to cut through a cell array without a loop, like
s{1:length(s)}.name?


Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

siko1056
On Fri, Dec 7, 2018 at 9:21 PM Przemek Klosowski <[hidden email]> wrote:
On 12/7/18 2:28 PM, Rik wrote:
> There is a new bug report #55173 which which shows, possibly, an irrelevant
> warning.
>
> octave:1> load penny.mat
> warning: load: '/usr/local/share/octave/5.0.0/data/penny.mat' found by
> searching load path
>
> I can see two approaches.  First, this warning has an ID,
> Octave:data-file-in-path, and we could change the default on Octave startup
> to disable this warning.  At that point, a user would have to actively turn
> on this warning to see it.
>
> Alternatively, I'm not sure why it is useful to know that the data file was
> found in the load path.  We could just remove the code entirely.
>
> Any votes one way or the other?

On the face of it, the warning is silly ("2+2 :  warning: addition:
result 4 found by applying the rules of arithmetic"). I think the intent
might have been to show the actual location of the file: I could see how
it might be useful to capture that, if 'load' allowed 2 return parameters :

[x,f]=load('penny.mat')

where x is the dataset being read and f is the filename.

All output should be catchable : it's neat that you can do

s=pkg('list')

for i=1:length(s)   printf("%s: %s\n",s{i}.name,s{i}.description)   end

I thought there were instances where this was not the case (I seem to
remember that pkg didn't always provide a way to capture the output),
but I can't recall where I ran into it.


BTW, is there a way to cut through a cell array without a loop, like
s{1:length(s)}.name?



I vote for simply removing this warning (i.e. Riks second option).  The only useful scenario I can imagine is, when you misspell your data file in the current directory, say

octave:1> load penny.mat

instead of

octave:1> load Penny.mat

that the warning might be helpful to detect that a totally different file is loaded.  But this is not a convincing use case to me.  Neither in the repo, nor the changelogs I found a convincing reason why they exist.

Kai


Regarding the BTW.  In this case it seems harder, as the underlying structs are not sorted by the fieldnames.
You might try this (not really more beautiful than the loop):

s = pkg ("list");
cdata = [cellfun(@(x) getfield (x, "name"), s, "UniformOutput", false);
             cellfun(@(x) getfield (x, "description"), s, "UniformOutput", false)];
printf("%s: %s\n",cdata{:})

Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

Carnë Draug
In reply to this post by Rik-4
On Fri, 7 Dec 2018 at 19:29, Rik <[hidden email]> wrote:

>
> There is a new bug report #55173 which which shows, possibly, an irrelevant
> warning.
>
> octave:1> load penny.mat
> warning: load: '/usr/local/share/octave/5.0.0/data/penny.mat' found by
> searching load path
>
> I can see two approaches.  First, this warning has an ID,
> Octave:data-file-in-path, and we could change the default on Octave startup
> to disable this warning.  At that point, a user would have to actively turn
> on this warning to see it.
>
> Alternatively, I'm not sure why it is useful to know that the data file was
> found in the load path.  We could just remove the code entirely.
>
> Any votes one way or the other?

There's a third option.  There could be a variable that specifies the
data directory for Octave.  So someone could use:

    load (fullfile (OCTAVE_DATA_DIR, "penny.mat"));

And be explicit about what they want.  But since this is really only
for Octave's data, maybe a function instead?  Something like this:

    load_octave_data ("penny.mat");
    load_demo_data ("penny.mat");


The warning seems fine to me.  Searching the path for a file with the
same name and ignoring file extension is the type of feature that I
think we only support for Matlab compatibility.  But it's so crazy
that a warning against such terrible practice is justified.

The issue is that there's no way to load data files distributed with
Octave without triggering the warning (or without going through the
private __octave_config_info__).

Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

John W. Eaton
Administrator
On 12/10/18 10:11 AM, Carnë Draug wrote:

> There's a third option.  There could be a variable that specifies the
> data directory for Octave.  So someone could use:
>
>      load (fullfile (OCTAVE_DATA_DIR, "penny.mat"));
>
> And be explicit about what they want.  But since this is really only
> for Octave's data, maybe a function instead?  Something like this:
>
>      load_octave_data ("penny.mat");
>      load_demo_data ("penny.mat");

The directory name is already in the struct returned by
__octave_config_info__.  On IRC, you pointed out that it's probably bad
form to ask users to access this info using a "private" function and I
agree.  We recently renamed the "public" octave_config_info function to
the "private" name __octave_config_info__ because a lot of that info is
really internal to Octave's build configuration.  But some of it is
useful and might be made public.  So we could split this into a public
and private function.  That might be better than having a special
function just for loading Octave demo data files.

> The warning seems fine to me.  Searching the path for a file with the
> same name and ignoring file extension is the type of feature that I
> think we only support for Matlab compatibility.  But it's so crazy
> that a warning against such terrible practice is justified.

Yes, this is the reason that I added this warning.  Do file reading
functions in other languages search a path for files?  That seems like a
strange feature to me that can lead to unpredictable results.  It also
seemed bad to me that the path used to search for data files was the
same as the one used for searching for program (.m) files.

For those who want more faithful Matlab compatibility, there is the
"--braindead^H^H^H^H^H^H^H^H^Htraditional" option.  This warning is
already disabled when invoking Octave with that option.

I could be persuaded that the warning should always be disabled by
default, but please don't remove it.

jwe

Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

Rik-4
On 12/10/2018 08:32 AM, John W. Eaton wrote:

> On 12/10/18 10:11 AM, Carnë Draug wrote:
>
>> There's a third option.  There could be a variable that specifies the
>> data directory for Octave.  So someone could use:
>>
>>      load (fullfile (OCTAVE_DATA_DIR, "penny.mat"));
>>
>> And be explicit about what they want.  But since this is really only
>> for Octave's data, maybe a function instead?  Something like this:
>>
>>      load_octave_data ("penny.mat");
>>      load_demo_data ("penny.mat");
>
> The directory name is already in the struct returned by
> __octave_config_info__.  On IRC, you pointed out that it's probably bad
> form to ask users to access this info using a "private" function and I
> agree.  We recently renamed the "public" octave_config_info function to
> the "private" name __octave_config_info__ because a lot of that info is
> really internal to Octave's build configuration.  But some of it is
> useful and might be made public.  So we could split this into a public
> and private function.  That might be better than having a special
> function just for loading Octave demo data files.

I'd prefer not to have another function.  If we know OCTAVE_DATA_DIR,
couldn't we simply check whether the file we found was in that directory
and skip producing a warning in that case?  The code is in utils.cc.

        if (! local_file_ok)
          {
            load_path& lp =
              __get_load_path__ ("find_data_file_in_load_path");

            // Not directly found; search load path.
            std::string tmp
              = sys::env::make_absolute (lp.find_file (fname));

            if (! tmp.empty ())
              {
                warn_data_file_in_path (fcn, tmp);

                fname = tmp;
              }
          }

If the file is found through searching (! tmp.empty ()), add in another
test that the directory is not the same as OCTAVE_DATA_DIR.

--Rik

>
>> The warning seems fine to me.  Searching the path for a file with the
>> same name and ignoring file extension is the type of feature that I
>> think we only support for Matlab compatibility.  But it's so crazy
>> that a warning against such terrible practice is justified.
>
> Yes, this is the reason that I added this warning.  Do file reading
> functions in other languages search a path for files?  That seems like a
> strange feature to me that can lead to unpredictable results.  It also
> seemed bad to me that the path used to search for data files was the same
> as the one used for searching for program (.m) files.
>
> For those who want more faithful Matlab compatibility, there is the
> "--braindead^H^H^H^H^H^H^H^H^Htraditional" option.  This warning is
> already disabled when invoking Octave with that option.
>
> I could be persuaded that the warning should always be disabled by
> default, but please don't remove it.
>
> jwe
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

John W. Eaton
Administrator
On 12/10/18 3:25 PM, Rik wrote:

> I'd prefer not to have another function.  If we know OCTAVE_DATA_DIR,
> couldn't we simply check whether the file we found was in that directory
> and skip producing a warning in that case?  The code is in utils.cc.
>
>          if (! local_file_ok)
>            {
>              load_path& lp =
>                __get_load_path__ ("find_data_file_in_load_path");
>
>              // Not directly found; search load path.
>              std::string tmp
>                = sys::env::make_absolute (lp.find_file (fname));
>
>              if (! tmp.empty ())
>                {
>                  warn_data_file_in_path (fcn, tmp);
>
>                  fname = tmp;
>                }
>            }
>
> If the file is found through searching (! tmp.empty ()), add in another
> test that the directory is not the same as OCTAVE_DATA_DIR.

To me, the situation is no different if the file is found in
OCTAVE_DATA_DIR or somewhere else.  If I happen to have my own file
penny.mat but am confused about what directory I'm in, I'd like that to
be an error rather than silently loading Octave's penny.mat file.

It's fine with me if we disable the warning by default, but I'd like to
keep it.

jwe

Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

Przemek Klosowski-7
On 12/10/18 3:33 PM, John W. Eaton wrote:
            load_path& lp =
               __get_load_path__ ("find_data_file_in_load_path");

             // Not directly found; search load path.
             std::string tmp
               = sys::env::make_absolute (lp.find_file (fname));

             if (! tmp.empty ())
               {
                 warn_data_file_in_path (fcn, tmp);

                 fname = tmp;
               }
           }

If the file is found through searching (! tmp.empty ()), add in another
test that the directory is not the same as OCTAVE_DATA_DIR.

To me, the situation is no different if the file is found in OCTAVE_DATA_DIR or somewhere else.  If I happen to have my own file penny.mat but am confused about what directory I'm in, I'd like that to be an error rather than silently loading Octave's penny.mat file.

It's fine with me if we disable the warning by default, but I'd like to keep it.

I think it's possible to find out the location of the file that was loaded:  file_in_loadpath(fname) should find the same file as sys::env::make_absolute (lp.find_file (fname)), right?

In that case, I would disable the warning for myself---but I couldn't figure out how; I thought it'd be

warning('off','Octave:load-file-in-path')

but no:

octave:11> load x
warning: load: '/tmp/x' found by searching load path

Reply | Threaded
Open this post in threaded view
|

Re: File found by searching load path

Przemek Klosowski-7
On 12/11/18 12:29 PM, Przemek Klosowski wrote:

>
> In that case, I would disable the warning for myself---but I couldn't
> figure out how; I thought it'd be
>
> warning('off','Octave:load-file-in-path')
>
> but no:
>
> octave:11> load x
> warning: load: '/tmp/x' found by searching load path
>
Ian McCallion pointed out that I turned off the wrong warning:

[a,b]=lastwarn
a = load: '/tmp/x' found by searching load path
b = Octave:data-file-in-path

warning('off','Octave:data-file-in-path') indeed turns that warning off.