Skip to content

Use scientific notation for big values in labeled slider #226

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Czaki
Copy link
Contributor

@Czaki Czaki commented Dec 15, 2023

Closes #224

Use QLabel instead of QDoubleSpinBox for custom representation of big/small values.

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Attention: Patch coverage is 81.31868% with 17 lines in your changes missing coverage. Please review.

Project coverage is 86.80%. Comparing base (358d041) to head (83319e7).

Files with missing lines Patch % Lines
src/superqt/sliders/_labeled.py 81.31% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #226      +/-   ##
==========================================
- Coverage   87.14%   86.80%   -0.35%     
==========================================
  Files          49       49              
  Lines        3774     3835      +61     
==========================================
+ Hits         3289     3329      +40     
- Misses        485      506      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tlambert03
Copy link
Member

basic idea looks good. lemme know when ready for a full review. How do you plan to manage when it switches into scientific notation?

@Czaki
Copy link
Contributor Author

Czaki commented Dec 17, 2023

Currently, it depends on str(float), as for big values, on my machine it uses scientific notation. I need to check how universal it is.

@tlambert03
Copy link
Member

hey @Czaki, gentle ping on this PR. still interested in adding this?

@Czaki
Copy link
Contributor Author

Czaki commented Dec 14, 2024

Yes. But I'm not sure if I will find time before January (Christmas etc).

@tlambert03
Copy link
Member

K, no hurry. Enjoy

@jiban
Copy link

jiban commented Feb 5, 2025

Hi @Czaki,
Thank you for the implementation.
By the way, I've looked for an explicit specification that stated str(float) used scientific notation when necessary (i.e General Format, 'g')
I could not find that statement but at least I found that cpython implementation was working like that.
str(float) uses General Format by default.

Here are what I found.
I hope it helps you save time.

  1. From Python Doc. for str()

    str(object) returns type(object).__str__(object), which is the “informal” or nicely printable string representation of object. For string objects, this is the string itself. If object does not have a __str__() method, then str() falls back to returning repr(object).

    so str(float) will call float.__str__() or float.__repr__()

  2. Python float does not have __str__() but have __repr()__. See PyTypeObject PyFloat_Type.

  3. float_repr() which float.__repr__ is mapped to calls PyOS_double_to_string() with format_code of 'r'.

    static PyObject *
    float_repr(PyFloatObject *v)
    {
        PyObject *result;
        char *buf;
    
        buf = PyOS_double_to_string(PyFloat_AS_DOUBLE(v),
                                    'r', 0,
                                    Py_DTSF_ADD_DOT_0,
                                    NULL);
        if (!buf)
            return PyErr_NoMemory();
        result = _PyUnicode_FromASCII(buf, strlen(buf));
        PyMem_Free(buf);
        return result;
    }
  4. Finally, PyOS_double_to_string() uses 'g'(General Format) option for 'r' format code.

    case 'r':          /* repr format */
    /* Supplied precision is unused, must be 0. */
    if (precision != 0) {
        PyErr_BadInternalCall();
        return NULL;
    }
    /* The repr() precision (17 significant decimal digits) is the
       minimal number that is guaranteed to have enough precision
       so that if the number is read back in the exact same binary
       value is recreated.  This is true for IEEE floating point
       by design, and also happens to work for all other modern
       hardware. */
    precision = 17;
    format_code = 'g';
    break;

so, I think your choice of using str(float) is good.

Thank you.

-- Jake

@Czaki Czaki changed the title Use scientific notation for big values inl labeled slider Use scientific notation for big values in labeled slider May 6, 2025
@Czaki
Copy link
Contributor Author

Czaki commented May 6, 2025

@tlambert03 may you take a look on the implementation of setValue to tell what did you think about the logic to converting to string?

Maybe the minimum and maximum limits should be more flexible:

if opt == EdgeLabelMode.LabelIsRange:
self.setMinimum(-9999999)
self.setMaximum(9999999)
with contextlib.suppress(Exception):
self._slider.rangeChanged.disconnect(self.setRange)

Copy link
Member

@tlambert03 tlambert03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may you take a look on the implementation of setValue to tell what did you think about the logic to converting to string?

seems reasonable to me i think? Maybe a few more tests would help clarify whether it's doing what you want?

Maybe the minimum and maximum limits should be more flexible:

sure, seems reasonable

self.editingFinished.connect(self._silent_clear_focus)
self._update_size()

def _editig_finished(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo _editig_finished -> _editing_finished

@Czaki Czaki marked this pull request as ready for review May 28, 2025 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Use scientific notification for Labeled sliders values for big/small values
3 participants