Skip to content

Conversation

@kv9898
Copy link
Contributor

@kv9898 kv9898 commented Dec 11, 2025

Addresses #4818. I attempted to address this problem by #7266, but that was rejected for the reason that it is more desirable to let the backends control formatting options rather than a global config. This PR takes exactly the latter approach.

Note that this PR only adds an optional format_options field to the protocol, and the functionality requires corresponding changes to the backends to respect the new protocol. posit-dev/ark#987 implements this for ark (the R backend).

The combined end result looks like (using the code example in the original issue):
Image

Changes

Protocol Layer

  • Added optional format_options field to BackendState in data_explorer-backend-openrpc.json
  • Updated TypeScript and Python interfaces to include FormatOptions in backend state

Frontend Integration

  • Modified DataExplorerClientInstance.updateBackendState() to merge backend-provided format options with defaults
  • Added constants DEFAULT_DATA_THOUSANDS_SEP and DEFAULT_PROFILE_THOUSANDS_SEP
  • Preserved distinct thousands separator handling for data display ('') vs profiles (',')

Python Backend

  • Added _get_format_options() method to DataExplorerTableView base class for subclass override
  • Included format_options in BackendState returned by get_state()

Documentation

  • ARK_IMPLEMENTATION.md: Implementation guide for R backend with max_integral_digits = 7 + scipen calculation
  • positron/comms/data_explorer.md: Protocol documentation for format options

How It Works

Backends now return format options in their state:

interface FormatOptions {
  large_num_digits: 2,
  small_num_digits: 4,
  max_integral_digits: 7 + scipen,  // Controls scientific notation threshold
  max_value_length: 1000,
  thousands_sep?: string
}

Frontend merges backend options with defaults, falling back to max_integral_digits: 7 if not provided. This allows:

  • R backends to respect scipen (requires ark repository changes)
  • Python backends to respect pandas settings (future)
  • CSV viewer to use sensible defaults
  • Full backward compatibility

Release Notes

New Features

Bug Fixes

  • N/A

QA Notes

Risk Level: Low. Changes are additive and backward compatible.

Testing after ark implementation:

  1. Test R with default scipen=0: Numbers ≥10M display as scientific notation
  2. Test R with options(scipen=10): Larger numbers display in fixed notation
  3. Test R with options(scipen=-3): Smaller numbers display as scientific notation
  4. Verify Python and CSV viewer continue using defaults
  5. Run existing data explorer test suite

Adjacent areas: None. Format option merging is self-contained in updateBackendState().

Context: The ark repository needs updates to implement format_options calculation from R's scipen option. See ARK_IMPLEMENTATION.md for detailed guidance.

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Thank you all for this PR! We ask that you sign our Contributor License Agreement before we accept your contribution. You can sign the CLA by posting a comment on this PR saying:


I have read the CLA Document and I hereby sign the CLA


1 out of 2 committers have signed the CLA.
✅ (kv9898)[https://github.com/kv9898]
❌ @Copilot
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Copilot AI and others added 5 commits December 11, 2025 22:24
Co-authored-by: kv9898 <105025148+kv9898@users.noreply.github.com>
Co-authored-by: kv9898 <105025148+kv9898@users.noreply.github.com>
Co-authored-by: kv9898 <105025148+kv9898@users.noreply.github.com>
Co-authored-by: kv9898 <105025148+kv9898@users.noreply.github.com>
Co-authored-by: kv9898 <105025148+kv9898@users.noreply.github.com>
@kv9898 kv9898 marked this pull request as ready for review December 11, 2025 14:44
@kv9898
Copy link
Contributor Author

kv9898 commented Dec 11, 2025

@juliasilge I can't get copilot to sign the CLA, can you help me with this?

@kv9898
Copy link
Contributor Author

kv9898 commented Dec 17, 2025

recheck

@kv9898
Copy link
Contributor Author

kv9898 commented Dec 17, 2025

@juliasilge Hmm the same problem (Copilot not included in the allowlist) also appears here:

allowlist: DavisVaughan, dependabot[bot], dfalbel, isabelizimm, jonvanausdeln, lionel-, nstrayer, petetronic, positron-bot[bot], seeM, sharon-wang, softwarenerd, timtmok, wesm, posit-jenkins-enterprise[bot], github-actions[bot]

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