merenlab / anvio

An analysis and visualization platform for 'omics data
http://merenlab.org/software/anvio
GNU General Public License v3.0
415 stars 142 forks source link

[BUG] Search with operators in the interactive interface #2153

Open FlorianTrigodet opened 8 months ago

FlorianTrigodet commented 8 months ago

Short description of the problem

In the interactive interface, we can search for items matching an operator formula. And it does not work with large numbers somewhat.

anvi'o version

anvi'o v8

Detailed description of the issue

With this table: toto.txt

anvi-interactive --manual -d toto.txt -p PROFILE.db

The search works when I search for values > 70,000:

Screenshot 2023-10-23 at 15 45 22

But it fails when I search for values > 2,000,000, it reports too many things including values not matching my condition?:

Screenshot 2023-10-23 at 15 45 36

No idea what is happing here 🤷

meren commented 8 months ago

lol. weird indeed.

@metehaansever, can you please take a look at this?

Ge0rges commented 3 months ago

@FlorianTrigodet @metehaansever I was looking into this but couldn't reproduce it, has it been fixed and/or can you reproduce it?

Ge0rges commented 3 months ago

I will say that generally though something like this would be cleaner/safer than eval():

function evaluateCondition(cellValue, searchValue, operator, operatorText) {
    switch (operator) {
        case '0': // "=="
            return cellValue.toString() === searchValue;
        case '1': // "!="
            return cellValue.toString() !== searchValue;
        case '2': // ">"
            return parseFloat(cellValue) > parseFloat(searchValue);
        case '3': // "<"
            return parseFloat(cellValue) < parseFloat(searchValue);
        case '4': // ">="
            return parseFloat(cellValue) >= parseFloat(searchValue);
        case '5': // "<="
            return parseFloat(cellValue) <= parseFloat(searchValue);
        case '6': // "contains"
            return cellValue.toString().toLowerCase().includes(searchValue.toLowerCase());
        default:
            return; // or throw an error
    }
}
meren commented 3 months ago

Absolutely -- No eval's should ever come close to anywhere near user-provided data.

metehaansever commented 3 months ago

I realized that using the Eval function posed significant problems, so I replaced it with Function(). However, this didn't resolve the issue. I believe we should reconsider the entire structure from the ground up.