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

Fix HTML table display on Numpy 2 #4970

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

titodalcanton
Copy link
Contributor

@titodalcanton titodalcanton commented Dec 4, 2024

HTML tables in result web pages are broken with Numpy 2. This should fix it.

Standard information about the request

This is a bug fix.

This bug involves anything that makes HTML tables of data (this includes at least the all-sky offline search and PyGRB) but the fix not impact anything.

Motivation

The problem is described in #4968, but to summarize, HTML tables show up empty under Numpy 2 because the data array in JavaScript ends up being filled with strings like 'np.float64(7.1)' instead of proper numbers like 7.1.

Contents

I added an explicit conversion of Numpy's float and integer types to Python native floats and ints respectively, before they are passed to the template that renders them into JavaScript values.

Links to any issues or associated PRs

Fixes #4968.

Testing performed

I used the following code:

import numpy as np
from pycbc.results import html_table

th = ['int', 'float', 'string', 'uint32', 'float32', 'complex128']
td = [
    np.array([1, 2, 3, 4]),
    np.array([1.1, 2.2, 3.3, 4.4]),
    np.array(['a', 'b', 'c', 'xyz']),
    np.array([1, 2, 3, 4], dtype=np.uint32),
    np.array([1.1, 2.2, 3.3, 4.4], dtype=np.float32),
    np.array([10+1j, 20+2j, 30+3j, 40+4j], dtype=np.complex128)
]
ht = html_table(td, th, page_size=20)
file_name = f'table_numpy_{np.__version__}.html'
with open(file_name, 'w') as tf:
    tf.write(ht)

Before this fix, this produces HTML files that actually show up blank with both Numpy 1.23.5 and 2.1.2, i.e. this test uncovered an additional bug with complex arrays breaking the tables (admittely not a likely use case though):

http://leela.fisica.indivia.net/~tito/pycbc/test_html_tables_numpy_2/table_before_fix_numpy_1.23.5.html

http://leela.fisica.indivia.net/~tito/pycbc/test_html_tables_numpy_2/table_before_fix_numpy_2.1.2.html

After this fix, I get identical-looking tables with both Numpy versions:

http://leela.fisica.indivia.net/~tito/pycbc/test_html_tables_numpy_2/table_after_fix_numpy_1.23.5.html

http://leela.fisica.indivia.net/~tito/pycbc/test_html_tables_numpy_2/table_after_fix_numpy_2.1.2.html

Additional notes

N/A

  • The author of this pull request confirms they will adhere to the code of conduct

@titodalcanton titodalcanton marked this pull request as draft December 4, 2024 17:49
@titodalcanton titodalcanton marked this pull request as ready for review December 5, 2024 09:19
Copy link
Contributor

@spxiwh spxiwh left a comment

Choose a reason for hiding this comment

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

Thanks, approved assuming it works, one minor optional comment/thought.

elif column.dtype.kind in 'iu':
row2.append(int(item))
else:
row2.append(str(item))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to just convert everything to string and avoid the type checks here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we convert floats and ints to string, they will show up in the JavaScript as '12.3' instead of 12.3, and we will be back to the same problem, i.e. JavaScript will see an array of strings instead of numbers.

@titodalcanton titodalcanton merged commit 59864d3 into gwastro:master Dec 5, 2024
29 checks passed
@titodalcanton titodalcanton deleted the fix-tables-in-numpy-2 branch December 5, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PyGRB's tables of injections seem to be broken under Numpy >2
2 participants