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 float32 JSON serialization #6

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Fix float32 JSON serialization #6

merged 6 commits into from
Jun 20, 2024

Conversation

serareif
Copy link
Collaborator

@serareif serareif commented Jun 19, 2024

Fix Error:

Traceback (most recent call last):
  File "/home/vscode/.local/lib/python3.10/site-packages/shiny/session/_session.py", line 665, in _run
    await flush()
  File "/home/vscode/.local/lib/python3.10/site-packages/shiny/reactive/_core.py", line 260, in flush
    await _reactive_environment.flush()
  File "/home/vscode/.local/lib/python3.10/site-packages/shiny/reactive/_core.py", line 178, in flush
    await self._flushed_callbacks.invoke()
  File "/home/vscode/.local/lib/python3.10/site-packages/shiny/_utils.py", line 528, in invoke
    await fn()
  File "/home/vscode/.local/lib/python3.10/site-packages/shiny/session/_session.py", line 1021, in _flush
    await self._send_message(message)
  File "/home/vscode/.local/lib/python3.10/site-packages/shiny/session/_session.py", line 960, in _send_message
    message_str: str = json.dumps(message) + "\n"
  File "/usr/local/lib/python3.10/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/usr/local/lib/python3.10/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/local/lib/python3.10/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/usr/local/lib/python3.10/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type float32 is not JSON serializable
**Object of type float32 is not JSON serializable**

The issue was resolved by globally converting all possible numerical values into a format that is compatible with JSON. This ensures that both existing and future values are always JSON-serializable, preventing any non-JSON-compatible values from causing errors.

@serareif serareif requested a review from nictru June 19, 2024 07:45
@serareif serareif self-assigned this Jun 19, 2024
@serareif serareif changed the title Sera/float issue Sera/float32_conversion_for_JSON Jun 19, 2024
@nictru nictru changed the title Sera/float32_conversion_for_JSON Fix float32 JSON serialization Jun 19, 2024
Copy link
Collaborator

@nictru nictru left a comment

Choose a reason for hiding this comment

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

The proposed solution is a symptomatic solution rather than a root-cause solution. I understand the problem only occurs in some AnnData objects; otherwise, we would have noticed it during earlier testing. This means that the structural differences should be solvable by converting the datatypes already when reading the input.

I understand that the monkey-patching looks rather elegant, but since json.dumps is a core functionality in networking, there might be negative side effects that we are unaware of - potentially just shifting the problem from one location to the other. Thus, I would not be comfortable merging this solution.

@serareif
Copy link
Collaborator Author

"I understand the problem only occurs in some AnnData objects; otherwise, we would have noticed it during earlier testing"
The problem occurs in all of the AnnData object which we used before for testing too. It also occurs in the commits beforehand for the same AnnData objects, which worked well before.
I will try to fix the issue by converting the datatypes beforehand.

@serareif serareif requested a review from nictru June 19, 2024 11:12
@nictru nictru merged commit ea1d4c8 into main Jun 20, 2024
1 check passed
@nictru nictru deleted the sera/float_issue branch June 20, 2024 05:58
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.

2 participants