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

Warn user about bad number_input format string #800

Closed
tconkling opened this issue Dec 4, 2019 · 3 comments
Closed

Warn user about bad number_input format string #800

tconkling opened this issue Dec 4, 2019 · 3 comments
Assignees
Labels
type:bug Something isn't working

Comments

@tconkling
Copy link
Contributor

tconkling commented Dec 4, 2019

This code:

import streamlit as st

st.number_input(
    label="input",
    format="a%.3f",
)

results in the NumberInput not showing any value. "a%.3f" is a valid format string, but it translates, e.g., a value of 0 into the string a0.000, which is not displayable by the NumberInput, which is restricted to numeric characters only.

We should probably detect this on the Python side (and warn the user) and/or clean up the format string (or resultant output) such that non-displayable characters won't result in nothing being displayed.

@tconkling tconkling changed the title Bad NumberInput format strings result in no value being displayed Warn user about bad number_input format string Dec 4, 2019
@tconkling tconkling added the type:bug Something isn't working label Dec 4, 2019
@nthmost nthmost self-assigned this Jan 3, 2020
@nthmost
Copy link
Contributor

nthmost commented Jan 3, 2020

I think the right behavior is to throw an error rather than cleaning up the input.

@nthmost
Copy link
Contributor

nthmost commented Jan 6, 2020

This is in the number_input react component -- currently, we're silently not-failing on whatever format weirdness may be afoot.

    try {
      return sprintf(format, value)
    } catch (e) {
      // Don't explode if we have a malformed format string.
      logWarning(`Error in sprintf(${format}, ${value}): ${e}`)
      return String(value)
    }
  }

@nthmost
Copy link
Contributor

nthmost commented Jan 7, 2020

Discussion point: we say we support a printf interface for formatting, but we honestly don't.

From st.number_input in the docs:

format (str or None) – A printf-style format string controlling how the interface should display numbers. This does not impact the return value.

Here are all the characters printf supports.

Stuff we don't support:

  • Uppercase formatting characters: The sprintf-js library we're using to format numbers doesn't support the uppercase versions of the formatters. Looks like sprintfjs doesn't either, though. (We could forcibly lowercase the format string in order to hack in support for uppercase characters.)

  • We don't support any of the hexadecimal formatters (%x, %X, %a, %A) because of early type-checking in DeltaGenerator.number_input. Here's the error when trying to input hexadecimal numbers through number_input(value=hex(2)):

TypeError: Both value and arguments must be of the same type. value has str type. min_value has NoneType type. max_value has NoneType type.

  • We also don't support pointer addresses (%n).

SUGGESTION... Maybe our documentation should list off the formatting characters we DO support, and support those explicitly. (It's a very finite list!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants