Data annotation tool 1 : Managing inputs and configuration

Question

I've started working on a data annotation tool to do segmentation on images or from a video file.

I decided to use Electron because I kind of like designing GUI with HTML/CSS and that it's very easy to set up.

Basically, the application as a limited set of functionality for now :

Select an input folder, video file or a single image;
Select an output folder, that isn't used yet;
Save the initial configuration so I don't have to redo it every time I reload the app;
Load images from a folder and load them in a canvas, where we can navigate between each images with previous/next buttons
Call a python script that parses a video and returns a path to an image file representing the current video, where we can also navigate between frames with previous/next (This might be a tad sketchy, but I don't mind as it works very well. I also don't want this reviewed, you may consider it as an external library).

This is the first part of the application, where I only deal with loading images and saving configuration, as you may see.

HTML

<!DOCTYPE html>
<html>
<head>
    <meta charset="UTF-8">
    <title>Hello World!</title>
    <meta http-equiv="Content-Security-Policy" content="script-src 'self' 'unsafe-inline' 'unsafe-eval';" />
    <style>
        label {
            display:block;
            padding:5px 0px;
        }
        .hidden {
            display:none;
        }
        .closed-pane {
            display:none;
        }
    </style>
</head>
<body>
    <div>
        <h2>Configuration</h2>
        <button class="toggle-pane configuration-pane-toggle">Toggle</button>
        <div class="configuration-pane">
            <ol>
                <li>Select which folder or file you want to load. You may load either a folder with images, a single image or a video file that will be parsed;</li>
                <li>Specify where you want the annotations to be saved. Note that if there already are annotations in this folder, they will be loaded;</li>
                <li>If loading a video, specify if you want to skip some frames as the annotation may get really long;</li>
                <li>If you don't want to redo this everytime you open the app, click "Save Config", a json file will be saved beside index.html</li>
                <li>Click Start and get started!</li>
            </ol>

            <label> Input : <input type="text" class="configuration-input"/> <button class="configuration-select-input">Select</button> </label>
            <label> Output : <input type="text" class="configuration-output"/> <button class="configuration-select-output">Select</button> </label>
            <label> Skip n frames : <input type="number" class="configuraiton-video-skip"/> </label>
            <button class="configuration-start">Start</button>
            <button class="configuration-save-config">Save config</button>
        </div>
    </div>
    <div>
        <h2>Annotation</h2>
        <div>
            Explanations : 
            <ol>
                <li>Press 1 to use the line annotator. You only need to click the beginning and the end of the line to create a segment. Then, use to scroll up/down to make the line bigger or smaller</li>
            </ol>
        </div>
        <div>
            <div class="navigation-pane">
                <button class="navigation-save-progress">Save</button>
                <button class="navigation-previous">Previous</button>
                <button class="navigation-next">Next</button>
            </div>
            
            <img id="current-image" class="hidden" width="1280" height="720" src=""/>

            <div class="canvas-container" style="position: relative;">
                <canvas id="image-canvas" width="1280" height="720"  style="position: absolute; left: 0; top: 0; z-index: 0;"></canvas>
                <canvas id="writer-canvas" width="1280" height="720"  style="position: absolute; left: 0; top: 0; z-index: 999;"></canvas>
            </div>               
        </div>
    </div>
</body>
<script src="config-io.js"></script>
</html>

JavaScript

main.js (Electron's entry point)

const { app, BrowserWindow } = require('electron');

function createWindow () {
  const win = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      nodeIntegration: true,
      enableRemoteModule: true
    }
  });
  win.setFullScreen(true);

  win.loadFile('src/index.html')
  win.webContents.openDevTools()

}

app.whenReady().then(createWindow)

app.on('window-all-closed', () => {
  if (process.platform !== 'darwin') {
    app.quit()
  }
})

app.on('activate', () => {
  if (BrowserWindow.getAllWindows().length === 0) {
    createWindow()
  }
})

config-io.js

const fs = require('fs');
const {dialog} = require('electron').remote;
const path = require("path");
const spawn = require("child_process").spawn;
const readline = require('readline');

const constants = Object.freeze({
    imageExtensions : ["png", "jpg", "jpeg"],
    videoExtensions : ["mp4"]
});

function isImage(f) {
    const ext = f.substring(f.lastIndexOf(".")+1);

    return constants.imageExtensions.includes(ext);
}

const html = Object.freeze({
    txtInput : document.getElementsByClassName("configuration-input")[0],
    btnChooseInput : document.getElementsByClassName("configuration-select-input")[0],
    btnChooseOutput : document.getElementsByClassName("configuration-select-output")[0],
    txtOutput : document.getElementsByClassName("configuration-output")[0],
    numSkipFrame : document.getElementsByClassName("configuraiton-video-skip")[0],
    btnSaveConfig : document.getElementsByClassName("configuration-save-config")[0],
    btnStart : document.getElementsByClassName("configuration-start")[0],
    btnPrevious : document.getElementsByClassName("navigation-previous")[0],
    btnNext : document.getElementsByClassName("navigation-next")[0],
    imgCurrent : document.getElementById("current-image"),
    cvsCurrentImg : document.getElementById("image-canvas")
});

const currentImageControler = function() {
    function init() {
        const file = html.txtInput.value;
        const fileName = file.substring(file.lastIndexOf(path.sep)+1);
        const isFile = fileName.lastIndexOf(".") != -1;
    
        if (isFile) {
            const extension = file.substring(file.lastIndexOf(".")+1);
            
            if (constants.videoExtensions.includes(extension)) {
                return fromVideo();
            } else if (constants.imageExtensions.includes(extension)) {
                return fromFile();
            } else {
                console.log("File not recognized");
                return;
            }
        }
        console.log("folder");
        return fromFolder();
    }
    
    const fromVideo = function() {
        const PythonFilePath = "Path to a script";
        let pythonProcess;
        let currentValue;

        const initialize = function() {
            pythonProcess = spawn('python3',[PythonFilePath, html.txtInput.value, html.txtOutput.value, html.numSkipFrame.value ?? 1]);
            pythonProcess.stderr.pipe(process.stderr);

            readline.createInterface({
                input: pythonProcess.stdout,
                terminal: false
            }).on('line', function(line) {
                currentValue = line;
                html.imgCurrent.src = currentValue;
            });
              
            html.btnNext.onclick = next;
            html.btnPrevious.onclick = previous;
            document.onclose = (_) => stop().bind(this);
        }

        function stop() {
            pythonProcess.kill();
        }

        function previous(e) {
            pythonProcess.stdin.write("back\n");
        }

        function next(e) {
            pythonProcess.stdin.write("next\n");
        }

        initialize();
    };

    const fromFolder = function() {
        let currentIndex;
        let files;

        const initialize = function() {
            fs.readdir(html.txtInput.value, {encoding : "utf8"}, function(err, res) {
                files = res.filter(isImage).map(x => html.txtInput.value + path.sep + x);

                console.log(files);
                currentIndex = 0;
                html.imgCurrent.src = files[currentIndex];

                html.btnNext.onclick = next.bind(this);
                html.btnPrevious.onclick = previous.bind(this);
            });
        }

        function next() {
            if (currentIndex >= files.length - 1)  return;

            currentIndex++;
            update();
        }

        function previous() {
            if (currentIndex == 0) return;

            currentIndex--;
            update();
        }

        const update = function() {
            html.imgCurrent.src = files[currentIndex];
        }

        initialize();
    };

    const fromFile = function() {
        let currentValue;

        const initialize = function() {
            currentValue = html.txtInput.value;
            html.imgCurrent.src = currentValue;
        }
        
        function current() {
            return currentValue;
        }

        initialize();
    }

    init();
}

window.onload = function() {
    if (!fs.existsSync("config.json")) return;

    const result = fs.readFileSync("config.json", {encoding: "utf-8"});
    const config = JSON.parse(result);
    html.txtInput.value = config.input;
    html.txtOutput.value = config.output;
    html.numSkipFrame.value = config.skip_n;
}

html.imgCurrent.onload = function() {
    var ctx = html.cvsCurrentImg.getContext("2d");
    ctx.drawImage(html.imgCurrent, 10, 10);
};

html.btnChooseInput.onclick = function() {
    dialog.showOpenDialog({properties: ['openDirectory', 'openFile']}).then(function (response) {
        if (response.canceled) return;

        html.txtInput.value = response.filePaths[0];
    });
};

html.btnChooseOutput.onclick = function() {
    dialog.showOpenDialog({properties: ['openDirectory', 'createDirectory'] }).then(function (response) {
        if (response.canceled) return;
        
        html.txtOutput.value = response.filePaths[0];
    });
};

html.btnStart.onclick = function() {
    currentImageControler();
    document.getElementsByClassName("configuration-pane")[0].classList.toggle("closed-pane");
};

html.btnSaveConfig.onclick = function() {
    const config = {
        input : html.txtInput.value,
        output : html.txtOutput.value,
        skip_n : html.numSkipFrame.value
    };

    const jsonConfig = JSON.stringify(config);
    fs.writeFile("config.json", jsonConfig, ["utf-8"], () => alert("saved"));
};

Array.from(document.getElementsByClassName("toggle-pane")).forEach(function(element) {
    element.addEventListener('click', function(e) { e.target.nextElementSibling.classList.toggle("closed-pane"); });
});

The whole thing works, but I'm pretty bad when it comes to structuring Javascript code. I come from an object oriented background, but I've been told countless times that while Javascript supports objects, it's not exactly the best paradigm to deal with what Javascript is.

So, I would like some feedback on the way my code is structured, I don't like how everything is laid out, I feel like it's messy.

CertainPerformance · Accepted Answer · 2020-11-13 18:27:28Z

Style rules You have a few rules in a <style> tag, but you also have some in inline attributes. To be consistent, better to put it all in the <style> tag. For example, change

<div class="canvas-container" style="position: relative;">

to

<div class="canvas-container">

.canvas-container {
  position: relative;
}

Destructure when you can, it's nice and concise (and can reduce the likelihood of typo-based problems). You're doing it in some places, but not others. const spawn = require("child_process").spawn; can be const { spawn } = require("child_process");.

Object.freeze is shallow You have:

const constants = Object.freeze({
    imageExtensions : ["png", "jpg", "jpeg"],
    videoExtensions : ["mp4"]
});

This will prevent the parent constants from having properties added or removed, but will not prevent the values from being mutated (which is different from reassignment). For example, someone could do constants.imageExtensions.push('BUG').

I'm not sure what the Object.freeze here is intended to do. If it's meant to show that the object shouldn't be changed at all, consider using the naming convention for absolute constants instead, with all caps:

const IMAGE_EXTENSIONS = ["png", "jpg", "jpeg"];
const VIDEO_EXTENSIONS = ["mp4"];

or use regular expressions (whose patterns are immutable).

Use querySelector to retrieve a single element If you only want the first element of a collection, then rather than use a method that returns a collection and selecting the [0]th element of the collection, better to use querySelector which will return the first matching element.

txtInput : document.getElementsByClassName("configuration-input")[0],

can be

txtInput : document.querySelector(".configuration-input"),

Another benefit of using querySelector by default is that it accepts a selector string, and selector strings are incredibly flexible, much more so than any other selection method. (They also match up exactly with CSS rules.)

To select an element with an ID, prefix it with #, eg document.querySelector('#current-image').

Precise variable names help prevent bugs.

Not a variable, but configuraiton is misspelled - easy source of bugs. Could fix the spelling, or shorten it (and all similar elements) to config.
I'd expect a variable named file to be a File object. If you just have a string, use something like fileName or pathToFile.

Or, even better, to get the file or folder name and extension, use path methods instead of manipulating the string manually. Use the .basename method:

const filePath = html.txtInput.value;
const baseName = path.basename(filePath);
const extension = path.extname(filePath);

Logs are for debugging, not for an ordinary user to read - if the file isn't recognized as an image or video, better to display that as an error message by putting it into an element on the screen, eg

<div class='error-file-not-recognized hidden'>
  File extension not recognized (Allowed extensions are: ...)
</div>

document.querySelector('.error-file-not-recognized').classList.remove('hidden');

Unnecessary variables and parameters In fromVideo, you have a persistent outer scoped variable currentValue, but it's only used in the line callback. I'd remove the variable entirely and change to:

}).on('line', function(line) {
    html.imgCurrent.src = line;
});

You aren't using the current function in fromFile either, nor its associated currentValue.

You also don't need to declare a parameter that isn't going to be used. Eg

function previous(e) { can be function previous() {, and document.onclose = (_) => can be document.onclose = () =>.

Using _ as a parameter name is a common convention when you need a later parameter but don't need the initial parameter(s), eg:

const arr = Array.from(
  { length: 5 },
  (_, i) => i
);

But since you aren't using later parameters, you can empty the argument list entirely.

bind fn.bind(this) is only useful when fn references this inside, and you need to make sure the calling context is preserved. But the stop function doesn't reference this, so it's not needed. You can use document.onclose = stop;. Same thing applies to the fromFolder function, which has the same issue in a couple places.

Assign immediately with const Rather than declaring a variable with let and then reassigning it inside a function, consider declaring the variable with const and assigning it a value immediately:

const pythonProcess = spawn('python3', // ...

Don't ignore errors Some of the callback-based functions you're calling return an error as the first parameter. Don't ignore this - the whole reason the error is the first parameter rather than a later one is to prompt the programmer to check for errors before going on to parse the result. When an error occurs, inform the user by changing an element in the DOM (not by just console.logging) so they can try again or send a bug report or something, rather than the script failing silently.

The same thing applies to the uses of dialog, though they're Promise-based, not callback-based, so to catch errors, add a .catch onto the end of them.

On a somewhat similar note, to make it clearer to the user what the script is currently doing, when fromVideo or fromFolder is called, consider setting the imgCurrent.src immediately (maybe to a placeholder like "Please wait for the image to load..."), and then set it again when the line or readdir callback runs.

Use const whenever you can. You're already doing this in most places, but you also have a spare var: var ctx = html.cvsCurrentImg.getContext("2d");. Consider using ESLint and no-var.

Concise iteration Instead of:

Array.from(document.getElementsByClassName("toggle-pane")).forEach(function(element) {

consider using querySelectorAll, whose NodeLists have forEach:

document.querySelectorAll('.toggle-pane').forEach((element) => {

Or invoke the iterator:

for (const element of document.querySelectorAll('.toggle-pane')) {

Overall This general structure of lots of somewhat-related event handlers being placed in the JS willy-nilly is pretty common with vanilla DOM manipulation. It's not bad per se, but it does look somewhat messy, and gets much worse the larger the page gets. If it were me, I'd strongly prefer to use a framework like React instead, removing the need to select elements from the DOM, allowing listeners to be tied directly with their associated DOM elements, and with better state management and more functional logic.

If you have time to learn (if you already know how JS works, it's pretty easy), IMO it makes larger projects much easier to manage, and is a lot nicer than vanilla DOM manipulation.

Thank you for your answers and suggestions, I appreciate it :) I'll look into React.JS — IEatBagels
– IEatBagels, Commented Nov 20, 2020 at 14:46

Stack Exchange Network

Data annotation tool 1 : Managing inputs and configuration

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Data annotation tool 1 : Managing inputs and configuration

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions