Issue with formatter?

[cyrinux]

Hi guys,

Seems there is an issue in the formatter.

I was a problem with the example above, while talking with @maximbaz, we find a solution but we don't understand it.

When doing conditional in format with external_script module.

By habit I use [] arround my format, but this is not working, this always show me the {output}: eg:

external_script copyq_count {                                                                                                                                                                               
 cache_timeout = 3600                                                                                                                                                                                      
 strip_output = True                                                                                                                                                                                       
 script_path = "copyq count 2>/dev/null"                                                                                                                                                                   
 format = '[\?if=output>210&color=degraded o {output:.0f}]'                                                                                                                                                  
 on_click 1 = 'exec py3-cmd refresh external_script copyq_count'                                                                                                                                           
}   

while this is working:

external_script copyq_count {                                                                                                                                                                               
 cache_timeout = 3600                                                                                                                                                                                      
 strip_output = True                                                                                                                                                                                       
 script_path = "copyq count 2>/dev/null"                                                                                                                                                                   
 format = '\?if=output>210&color=degraded o {output:.0f}'                                                                                                                                                  
 on_click 1 = 'exec py3-cmd refresh external_script copyq_count'                                                                                                                                           
}   

There is a reason? Thanks! /cc @tobes

[maximbaz]

One confusion was about square brackets, as mentioned above or in this example:

why this works:
  format = '\?color=path HELLO'
but this doesn't:
  format = '[\?color=path HELLO]'

another was about the order of expressions:

why this works:
  format = '\?color=degraded \?if=path HELLO'
but this doesn't:
  format = '\?if=path \?color=degraded HELLO'

[lasers]

The second one is inside the block, but does not have a placeholder.

• With a placeholder... format = '[\?color=path {name}]'
• Without a placeholder... format = '[\?color=path&show HELLO]'.

If {name} is '', False, or None, then the block will not be visible. Without a placeholder, do we want it visible? Definitely... so we add \?show to make the block condition True.

You should look at examples in /tests/test_formatter.py. You should use this... \?if=path&color=degraded HELLO. Without looking into why this happened, both example formats are confusing enough and we don't know the value of path so I assume None... which could make the second format disappear. The first format might work because there is no beginning \?if= condition to check.

[maximbaz]

This is good, makes sense now 🙂 Does this also mean that \?show and \if= are mutually exclusive?

[lasers]

Pretty much. \?if= is a condition. \?show is a True condition. The only time we need \?show is if there is no condition to work with.

• Don't have to add \?show in [\?if=name YES|NO].
• Need to add \?show in [{name}|\?show NO]

[cyrinux]

Thanks @lasers ! 👍

[lasers]

@cyrinux I believe you found a formatter bug. Please leave this open.

This bug seems to be happening because it didn't try to cast the string value to int, float, etc.

Not okay.         |  Okay.
>>> '21' > '210'  |  >>> 21 > 210
False             |  False
>>> '22' > '210'  |  >>> 22 > 210
True              |  False

EDIT:

diff --git a/py3status/modules/static_string.py b/py3status/modules/static_string.py
index dbcec8c6..974b3aed 100644
--- a/py3status/modules/static_string.py
+++ b/py3status/modules/static_string.py
@@ -16,12 +16,19 @@ class Py3status:
     """
     """
     # available configuration parameters
-    format = 'Hello, world!'
+    format = '\?if=output>210 OUTPUT'

     def static_string(self):
+        data = {'output': 22}        # print []  # OK
+        # data = {'output': '22'}    # print OUTPUT  # Not OK.
+        # data = {'output': '21'}    # print []  # OK
+        # data = {'output': ''}      # print []  # OK
+        # data = {'output': 'True'}  # print OUTPUT   # Not OK.
+        # data = {'output': True}    # print []    # OK.
+
         return {
             'cached_until': self.py3.CACHE_FOREVER,
-            'full_text': self.py3.safe_format(self.format),
+            'full_text': self.py3.safe_format(self.format, data),
         }

[cyrinux]

Hi guys ;) Was just facing yet to the problem. I forget it was here. Was trying to do another little module. I must compare two placeholders containing int or float in my format. Seems this doesn't work. If I can help / try say me ;)

[cyrinux]

His the bug here in those/except ? I understand value come as 'str' and are not try to be cast as int or float? Is it? https://github.com/ultrabug/py3status/blob/921b8f8b28a21de8196f5ff06ffa806c52eacfc0/py3status/formatter.py#L359

[lasers]

Maybe yes. The bug could be somewhere in that area. I recall the issue is that it relies on the incoming value or variable type, then cast both the same thing right away... instead of maybe trying to cast each individually and see if it works... okay.

If this is for speedtest-cli, we could int() or float() your values before the safe_format.

[lasers]

@cyrinux I used this branch several weeks ago to track down the problem. I hope you find it helpful.

• https://github.com/lasers/py3status/tree/formatter-bug

[cyrinux]

Thanks for branch. About cast before safe_format this should fix the problem?

[cyrinux]

@lasers I have try casting before safe_format, seems my < > check return always False on float. Will try to put nose deeper in formatter.

[tobes]

If there is a perceived bug can it be explained. with an example safe_format() call

[tobes]

@lasers can we close this - from what I can see all is good

[lasers]

@tobes There is a comparison bug if we don't try to cast strings to int/floats first before passing in the safe_format. You can try the examples https://github.com/ultrabug/py3status/issues/1391#issuecomment-404502160. For instance, '22' > '210' is wrong.

[tobes]

'22' > '210' == True changing this to 22 > 210 == False is wrong auto casting is really dangerous. If module writers cannot cast correctly this is not the formatter's problem.

We do cast to an extent because we need to but we cast to the type we are comparing with so '22' > 210 would be the wrong thing to do. All you examples give the correct result. If you find a comparison that the python interpreter disagrees with then that would be a bug but all these examples look good to me

[lasers]

This comment is much better and more descriptive than the recent comment.

It clears up confusions too. I wasn't sure if you saw what I saw... or you saw what I saw and chalks it up to that message above. I always wanted to be sure that we're looking at same thing. :slightly_smiling_face:

If I recall it correctly, this problem is in external_script. I take it we should make a PR to cast it there too? However, I wonder if it is more efficient to have modules always trying to cast everything in loops (on a off chance that there will be a comparison) via having safe_format always trying to cast it first (when there is an actual comparison)?

I don't know if there is a speedtest issue or formatter comparison issue (here) as it seems like @cyrinux is trying to do something... because he's casting everything to float. Few values are int.

[tobes]

;external_script is an interesting case but I think it is the only one that is a legitimate issue. Here we do not know if we have '2' or 2 because the shell does not tell us the difference.

So we could try to guess but I think this should be part of this module only. My take is if the output looks like an int we treat it as such same with a float if it is quoted ' or " it is always a str. But what about False, false? Truthiness is always fine but false is harder. I think I'd leave this because we can do \?if=!output=false I'd also be happy to ignore leading/trailing whitespace

How does that sound?

I wonder if it is more efficient to have modules always trying to cast everything in loops

this is the path to insanity - zen of python: Explicit is better than implicit.

because he's casting everything to float.

yes that is exactly how it should be done

[lasers]

I also wonder about running into different types... eg none, 'None', etc. When we run commands and things are not explicitly available or supported, sometimes they will have different output than None.

In nvidia_smi, I convert all [Not Supported] to None (string) because it's not possible to use NoneType at that time... and I could do if value == '[Not Supported]' then I'm always checking this for all values instead of doing it once with str.replace('[Not Supported]', 'None').

I'll go ahead and make a PR for external_script. Maybe with ast.literval_eval() ?

[tobes]

convert all [Not Supported] to None (string)

yeah that feels bad

I'll go ahead and make a PR for external_script

I'm ahead of you - I want to close all the bug issues

[tobes]

This should new be resolved in master