Skip to content
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

d3.min and d3.least don't always ignore nulls #3790

Closed
helderdarocha opened this issue Oct 28, 2023 · 4 comments
Closed

d3.min and d3.least don't always ignore nulls #3790

helderdarocha opened this issue Oct 28, 2023 · 4 comments

Comments

@helderdarocha
Copy link

The documentation states, for d3.min, that "Unlike the built-in Math.min this method ignores undefined, null and NaN values", so I would expect that if I had an array:

array = ['5', '10', null, '2']

then d3.min(array) would return '10' (which is expected, since natural order is used), and d3.min(array, d => +d) would return 2.

But no, d3.min(array) returns 0 (as expected, because null coerces to zero before the accessor function is called, but the user that expects nulls to be ignored might not care to verify this). Although the issue is actually in the conversion, this is easy to overlook and can lead to bugs.

The documentation for d3.least doesn't explicitly mention that it ignores null, but in this example, d3.least(array, d => +d) returns null (shouldn't it produce the same result as d3.min when using an accessor?)

@mbostock
Copy link
Member

Screenshot 2023-10-28 at 8 49 37 AM

You’re probably using an old version of D3?

@mbostock
Copy link
Member

As for the other cases, this:

d3.min(['5', '10', null, '2'], (d) => +d)

is equivalent to this:

d3.min(['5', '10', null, '2'].map((d) => +d))

is equivalent to this:

d3.min([5, 10, 0, 2])

hence it returns zero.

And likewise for d3.least, it doesn’t care that the element is null, it cares whether the return value of the given accessor is null. So if you coerce null to zero in your accessor, and zero is the least value, then d3.least will return the corresponding element which is null.

@helderdarocha
Copy link
Author

Yes. Thank you. I know this is the expected behavior. The problem is that the accessor is not really being used to access anything, but to coerce and change values in the dataset, which can lead to cryptic code. Mislead by an example of mine where I used +d to coerce strings, my student spent a day trying to fix a bug and fixed it after removing nulls from her dataset. She had understood that extent should ignore nulls, but wasn't aware that they were being coerced.

@mbostock
Copy link
Member

Yes, I recommend that you apply type coercion as early as possible — ideally when the data is loaded, say by using d3.autoType in conjunction with d3.csv, or Observable’s FileAttachment helper with typed: true. If you defer type coercion until later, there is much more surface area for these problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants