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

Should force_overwrite default to true? #161

Closed
nsheff opened this issue Feb 22, 2024 · 3 comments
Closed

Should force_overwrite default to true? #161

nsheff opened this issue Feb 22, 2024 · 3 comments
Labels
likely-solved priority-high question Further information is requested
Milestone

Comments

@nsheff
Copy link
Contributor

nsheff commented Feb 22, 2024

Right now, report has a force_override parameter, which defaults to False.

def report(
self,
values: Dict[str, Any],
record_identifier: str,
force_overwrite: bool = False,
result_formatter: Optional[staticmethod] = None,
) -> str:
_LOGGER.warning("Not implemented yet for this backend")

For me, it seems more natural that if I report() something, I would want it to replace whatever I had there most of the time. So, I think I would prefer if it defaults to True. (and maybe is renamed).

Open for discussion...

@nsheff nsheff added the question Further information is requested label Feb 22, 2024
@nsheff nsheff changed the title Should override default to true? Should force_overwrite default to true? Feb 22, 2024
@donaldcampbelljr
Copy link
Contributor

A counter argument: if it defaults to True, there is the potential for accidental data loss due to overwriting. I had assumed False was the default as a safety precaution.

@nsheff
Copy link
Contributor Author

nsheff commented Feb 22, 2024

the same argument goes in the other direction. without overwriting, you're losing the new result, and retaining the old one. So your safety precaution doesn't make sense, unless you prioritize old results over new ones.

In the past, I recorded them all.

This is the core issue raised here: databio/pypiper#209

@nsheff nsheff added this to the v0.9.0 milestone Mar 12, 2024
@nsheff nsheff added this to PEP Mar 12, 2024
@donaldcampbelljr
Copy link
Contributor

I flipped force_overwrite to default to True. I will do the same in PyPiper. However, we will still need to allow pipestat to offer the ability for history of results:

In the longer term, pipestat should offer the option to include a history of results, and these should be stored somehow in the file (and database). This may not actually be too hard to implement; just add a 'history' function, and when something is overwritten, just move the old values into the history in a way that is an array, rather than a single value. Then, pipestat could offer a clear history function to remove old stuff, if desired, but otherwise, repeated reports of the same result will simply add to the history.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
likely-solved priority-high question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants