Closing/Re-Opening to trigger CI run on our fancy new CI system.

Closing and re-opening (again) as I updated the CI one more time...

Hi @Hanwant sorry it's taken me a while to follow up on this. I've had my head in migrating CI platforms, which is more or less finished.

Thanks for the PR, I approve of the change, but I'm not feeling great about the +1 offset; there is value in looking at the log value of values > 0 but < 1 and see where they end up in the negative log space, but I agree that 0 value are problematic.

May I suggest we use .eps of the type of the array?

what I've done in my projects if something like this:

if np.issubdtype(y, np.floating):
    epsilon = np.finfo(y.dtype).eps
else:
    epsilon = 1
y = np.sign(y) * np.log10(np.abs(y) + epsilon)

What do you think?

Ah, I didn't have that use case in mind (interest in the 0<x<1 space) but certainly see its value. Perhaps an unintended consequence of this would be that values in the range -1 < x < 0 would now be positive after transformation, would that be acceptable?

I don't see an easy way to consolidate both use cases of scaling really large values and inspecting really small values aside from a check regarding the scale of y or two separate scaling functions. For visualizing the two cases, I've attached a graph showing the behaviour around 0. for the transform using eps and using +1.

Seeing as our personal use cases may differ, I guess it's a case of what other users prefer the default behaviour to be?

Nice plot, thanks for sharing!

Perhaps an unintended consequence of this would be that values in the range -1 < x < 0 would now be positive after transformation, would that be acceptable?

Didn't think about that, but I think that's fine. To take log of negative numbers, the library will have to do something, and it's not going to fit all use-cases; ideally the end user handles negative numbers on their own so the library doesn't have to, but I think sign * log10(abs(y) + epsilon) is completely fine.

I would advocate for using the np.finfo(y.dtype).eps method and adding a blurb in the docstring about the math involved.

Cool, sounds good. Have made the change and assumed the docstring was intended for setLogMode?

/home/runner/work/pyqtgraph/pyqtgraph/doc/source/../../pyqtgraph/graphicsItems/PlotDataItem.py:docstring of pyqtgraph.graphicsItems.PlotDataItem.PlotDataItem.setLogMode:3: WARNING: Definition list ends without a blank line; unexpected unindent.

This is the error the CI reported, I think it wants an empty line before/after scaled = sign(y) * log10(abs(y) + eps) (but I'm a noob when it comes to sphinx, so that's just a guess)

if it's not obvious to you, I'll hop on your branch and tinker with it locally as I have the doc build environment already setup

the macOS CI failure is an intermittent failure I haven't figured out a good way to deal with yet, it's unrelated to this PR.

Btw along with a '\n' I changed the indent from a tab to spaces, not sure which helped

oh, given it was a docstring, I was going to suggest, just adding extra new-lines

"""
        To enable log scaling for y<0 and y>0, the following formula is used:
        
            scaled = sign(y) * log10(abs(y) + eps)
        
        where eps is the smallest unit of y.dtype.
        This allows for handling of 0. values, scaling of large values,
"""

Oh right, that's clearer, updated again

Thanks for the patience, this was my first PR to an open source project.

Congratulations on your first PR! FWIW this is the first open source project I've been a maintainer of (that wasn't just some personal project).

If you see some other area that pyqtgraph could use some improvement in, and know how to to make the fix or want input, please reach out.

Thanks, merging.