search

modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
471 stars 168 forks source link

Change name to loadResource #3145

Open casella opened 4 years ago

casella commented 4 years ago

The choice of name of the Modelica.Utilities.Files.loadResource function is a bit misleading, since the function does not load anything anywhere, but simply converts a modelica:// URI into a string an absolute file path, that can be used as an input to the numerous functions that require a fileName input string.

As we have a once-in-a-lifetime opportunity to improve lame naming, I would propose to change its name from loadResource to uri2fileName or something similar.

I can take care of that if there is agreement that this is a good idea

beutlich commented 4 years ago

ModelicaServices.ExternalReferences.loadResource maps to Modelica.Utilities.Files.fullPathName by default. That's what I'd propose: fullPathName should consider modelica:// or file:// URIs.

henrikt-ma commented 4 years ago

I've always found it strange that loadResource isn't part of the language. Rather than just renaming the MSL function, I would suggest moving it to the specification as in this MCP suggestion. There, the language construct is called resolveURI.

HansOlsson commented 4 years ago

The loadResource is useful in tools (especially in 3D Experience platform, and also in Dymola) because it does more than merely giving a full path name/resolving the URI.

In particular we use it to project resources to the file-system (from the internal data-base) and also to copy references resources to FMIs. (Basically to create the file we return the path of.)

That's not an argument against clarifying the name nor against adding it to the specification; merely an indication that there is more to the story and uri2fileName or fullPathName also are problematic as names.

casella commented 4 years ago

ModelicaServices.ExternalReferences.loadResource maps to Modelica.Utilities.Files.fullPathName by default. That's what I'd propose: fullPathName should consider modelica:// or file:// URIs.

Could be a good idea, but then Modelica.Utilities.Files.fullPathName should call another ModelicaServices function. Isn't this a bit like ping-pong?

HansOlsson commented 4 years ago

Could be a good idea, but then Modelica.Utilities.Files.fullPathName should call another ModelicaServices function. Isn't this a bit like ping-pong?

As indicated above those two functions have different purposes, and we shouldn't mix them.

casella commented 4 years ago

The loadResource is useful in tools (especially in 3D Experience platform, and also in Dymola) because it does more than merely giving a full path name/resolving the URI.

In particular we use it to project resources to the file-system (from the internal data-base) and also to copy references resources to FMIs. (Basically to create the file we return the path of.)

Thanks @HansOlsson for pointing this out, I missed it!

The comment of the function is "Return the absolute path name of a URI or local file name", which is incomplete and thus a bit misleading. However, the documentation explains it better:

The function call "Files.loadResource(uri)" returns the absolute path name of the file that is either defined by an URI or by a local path name. With the returned file name it is possible to access the file with function calls of the C standard library. If the data or file is stored in a data-base, this might require copying the resource to a temporary folder and referencing that.

So the point is that the resource pointed by the URI may not be necessarily a file on the local file system, in which case calling the function creates a local copy and then makes it accessible via an absolute path name to that local copy. From this point of view, the loadResource name now actually makes sense to me.

What about just changing the comment to the existing function to "Makes the data referenced by a local path name or URI available in a file and returns its absolute path name"?

casella commented 4 years ago

And possibly rename it accessResource()?

HansOlsson commented 4 years ago

What about just changing the comment to the existing function to "Makes the data referenced by a local path name or URI available in a file and returns its absolute path name"?

That would be good.

And possibly rename it accessResource()?

I would be reluctant to do that; as it will imply a number of changes and it is not such an obvious improvement.

casella commented 4 years ago

Fine for me. If I get some more positive feedback, I can proceed with the commit.

henrikt-ma commented 4 years ago

Still, on all systems where the resource is already present in the file system, the load part of the name doesn't make much sense. The name would make a whole lot more sense if the function returned some kind of file handle instead of just a plain string…

beutlich commented 4 years ago

There, the language construct is called resolveURI.

That would be my preference.