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

Support list / tuple of strings for format #16

Open
nhairs opened this issue Jun 4, 2024 · 2 comments
Open

Support list / tuple of strings for format #16

nhairs opened this issue Jun 4, 2024 · 2 comments
Labels
enhancement New feature or request open for discussion Community feedback is wanted

Comments

@nhairs
Copy link
Owner

nhairs commented Jun 4, 2024

Many other JSON formatting libraries allow or directly use a list of strings for the format. This suggests that it might be worth us adding in support for it.

Pros:

  • No parsing means no testing for a correct format - we can directly use the list.
  • Allows for clearer config when using dictConfig with the () user-defined object indicator, or instantiating within Python itself.

Cons:

  • Must always use the () custom class indicator when using dictConfig.
  • Not compatible with fileConfig.
  • Not compatible with other formatters
@nhairs nhairs added enhancement New feature or request open for discussion Community feedback is wanted labels Jun 4, 2024
@FichteFoll
Copy link

Could you elaborate why it is not compatible with fileConfig? I assume because the file config format does not support deserialization of lists there?

@FichteFoll
Copy link

Okay, after checking the documentation a bit on this, since I've never used a fileConfig in the past, I've answerd the question myself.

  1. The handler's args are evaluated while the formatters are not
  2. Formatters are instead instantiated using special paramater names by default, or a kwargs dict if the custom method with key () is used
  3. These kwargs dicts values are parsed "as-is" and presumably using the configparser stdlib.
  4. configparser does not have a getlist method or similar.

docs:

I guess accepting a comma-separated string of the field names to include would be a decent workaround here but does not provide a significant enough reason to break compatibility with the current method.

Still, regarding the issue at hand, the constructor could simply be overloaded to also accept a list of strings and determine in code whether a list of fields was already provided or whether it must be extracted from a format string in the same format as a standard formatter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request open for discussion Community feedback is wanted
Projects
None yet
Development

No branches or pull requests

2 participants