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 serializing timedelta and relativedelta to string (hocon, json etc.) #263

Merged
merged 6 commits into from
Jul 12, 2021
Merged

Conversation

gabis-precog
Copy link
Contributor

Support serializing timedelta and relativedelta to string (hocon, json etc.)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.639% when pulling 5c741cc on gabis-precog:master into 07758b8 on chimpler:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.639% when pulling 5c741cc on gabis-precog:master into 07758b8 on chimpler:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.639% when pulling 5c741cc on gabis-precog:master into 07758b8 on chimpler:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.639% when pulling 5c741cc on gabis-precog:master into 07758b8 on chimpler:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.639% when pulling 5c741cc on gabis-precog:master into 07758b8 on chimpler:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 94.639% when pulling 5c741cc on gabis-precog:master into 07758b8 on chimpler:master.

@coveralls
Copy link

coveralls commented May 23, 2021

Coverage Status

Coverage increased (+0.8%) to 96.139% when pulling b7d6161 on gabis-precog:master into 07758b8 on chimpler:master.

@gabis-precog
Copy link
Contributor Author

Would you consider removing support for python 3.4 from tox as it is end-of-life instead of bothering to make it pass ?

@darthbear
Copy link
Member

Thanks @gabis-precog for your PR!

@darthbear darthbear merged commit 3d64330 into chimpler:master Jul 12, 2021
"""
if relativedelta is not None and isinstance(config, relativedelta):
if config.hours > 0:
return str(config.hours) + ' hours'
Copy link

@dolfinus dolfinus Jul 13, 2021

Choose a reason for hiding this comment

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

Pyhocon supports multiple input formats for timedelta, like:

1s
1 s
1 second
1 seconds
1 sec

But output format is fixed,and thus some applications expecting format other than this one will fail while parsing output config.

For example, Spark config values could contain time units like 150ms, but converter will save it as 150000 microseconds (there is just no case for milliseconds).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dolfinus
output is specifically for hocon format. if spark config is different then it is not hocon and requires it's own output. don't know if it's in the scope of this library.

Choose a reason for hiding this comment

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

I don't see here any specification about default unit format - all the options are described as supported by a parser. There is no preferred unit format, so why does implementation have one without any capability to change it whenever user wants?

Spark config is just the simple .properties file which is successfully read by any HOCON library implementation. But the output file saved by the current package cannot be changed at all - no class or instance options for setting up some preferred format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dolfinus
"so why does implementation have one without any capability to change it whenever user wants?" - because that is the implementation I submitted.

"Spark config is just the simple .properties" - so it's not hocon.

"file which is successfully read by any HOCON library implementation" - I do believe you are free to contribute code which enhances this library to your additional requirements. That's what I did.

ps.
From my perspective, you are coming off a bit demanding, considering this is open-source and based on free contributions from people who care enough to do so. less talk, more code :)

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.

4 participants