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

Clean up loader and _findPath functions and elucidate the need for prefix #423

Open
AntoineGautier opened this issue Jan 9, 2025 · 1 comment
Assignees

Comments

@AntoineGautier
Copy link
Collaborator

AntoineGautier commented Jan 9, 2025

The loader function uses a prefix string that is prepended to the list of directories containing Modelica JSON files:

export const MODELICAPATH = [
  `${config.MODELICA_DEPENDENCIES}/template-json/json/`,
];
...
const modelicaDirs = [prefix, ...MODELICAPATH];

Logging the value of prefix when executing npx ts-node scripts/parse-template-package.ts shows that it is either empty or equal to /dependencies/template-json/json/, which is already within MODELICAPATH.
So it seems that prefix can simply be removed.
The variable MODELICAPATH must also be renamed to avoid the confusion with the environment variable: https://github.com/lbl-srg/ctrl-flow-dev/blob/main/server/Dockerfile#L38

The function _findPath has an incorrect JSDoc and its argument reference seems to refer to a full class name. This should be made clearer.

@AntoineGautier
Copy link
Collaborator Author

AntoineGautier commented Jan 9, 2025

The confusion between file paths and class names spreads over several functions:

/**
 * Extracts the given file into the type store
 */
export const getFile = (filePath: string) => {
  const jsonData = loader(pathPrefix, filePath);
export class File {
  public package = ""; // only important part of a file
  public elementList: Element[] = [];
  constructor(obj: any, filePath: string) {
    this.package = obj.within;
    const splitFilePath = filePath.split(".");

All these functions expect a full class name (dot notation), not a relative file path.

The only occurrence where the path argument really refers to a file system path is here:
https://github.com/lbl-srg/ctrl-flow-dev/blob/main/server/src/parser/index.ts#L7-L17
But this function is called here:
https://github.com/lbl-srg/ctrl-flow-dev/blob/main/server/scripts/parse-template-package.ts#L12C1-L12C26
with "Buildings" as argument. So, this becomes confusing again because the JSDoc of loadPackage mentions @param packagePath Absolute path to package and "Buildings" is rather a directory that can be found just under the so-called MODELICAPATH in loader.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant