add toml support for ParamsDependency

[dtrifiro]

We've been testing DVC with a few of our projects and for some of those we use a toml config to store parameters.
This PR adds support for parameters file in toml format. Example:

dvc run -n train                      \
-d data/input.csv                   \
-o model/model.h5                \
-M model/train_history.csv   \
-p params.toml:model.training \
python train.py

Where params.toml looks something like this:

[transformers.numerical_quantile_transformer]
n_quantiles = 300
output_distribution = "uniform"

[model.training]
epochs = 50
verbose = 1

[skshetry]

We use yaml to load json as well.

[skshetry]

Could just try checking toml suffix, and then fallback to yaml.safe_load.

[dtrifiro]

I just force-pushed a check for suffix (use yaml.safe_load() for json and yaml)

[skshetry]

Still this misses the yml. So, it's better to just:

if suffix == "toml":
    # toml load
else:
   # yaml load

[pmrowla]

Is it actually safe for us to guess the file format from the suffix here? It seems like we'll potential to run into issues where users have params filenames with unexpected capitalization on case-sensitive filesystems, or where they are using .yml instead of .yaml.

It seems to me we should maybe just try to load as yaml (and json) first, then try toml if that fails, then error out?

[dtrifiro]

I added a loader map which is based on the lowercase file suffix, which defaults to yaml.safe_load for unknown suffixes. I also updated _read_params

[efiop]

I also agree with @pmrowla , it is better to just try to load and catch yaml formatting error than only trying to judge by the extension.

[dtrifiro]

In the last force-push I ended up using another approach (default to yaml.safe_load, use toml if lowercase suffix matches) because I felt a nested try/catch a bit awkward, especially if there are other loaders different from yaml/json and toml (not that I think there's much more, these 3 alone probably cover most of the use cases).

What is your preference?

Also, I saw that the Travis build fails on Windows and Mac (probably due to misconfiguration?) is there anything I need to do about that? Thanks

[skshetry]

@efiop, In case of unknown file format, sure, it could fallback to yaml; but, if the file is clearly a toml file, it should just try to load as toml and fail.

This is what @dtrifiro is doing.

[efiop]

Makes sense, let's keep it. 👍

[pared]

+1

[shcheklein]

Please, don't forget to update the docs after this is merged.

[codecov]

Codecov Report

Merging #4258 into master will decrease coverage by 0.47%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master    #4258      +/-   ##
==========================================
- Coverage   91.77%   91.30%   -0.48%     
==========================================
  Files         173      173              
  Lines       11743    11772      +29     
==========================================
- Hits        10777    10748      -29     
- Misses        966     1024      +58     

Impacted Files	Coverage Δ
dvc/external_repo.py	86.63% <94.44%> (+0.03%)	⬆️
dvc/scm/git.py	88.12% <100.00%> (+0.27%)	⬆️
dvc/system.py	62.39% <0.00%> (-26.50%)	⬇️
dvc/utils/fs.py	88.88% <0.00%> (-3.97%)	⬇️
dvc/analytics.py	92.59% <0.00%> (-3.71%)	⬇️
dvc/stage/run.py	94.02% <0.00%> (-2.99%)	⬇️
dvc/tree/ssh/connection.py	79.80% <0.00%> (-2.41%)	⬇️
dvc/daemon.py	54.38% <0.00%> (-1.76%)	⬇️
dvc/tree/local.py	93.86% <0.00%> (-0.95%)	⬇️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b77ce02...edebcb4. Read the comment docs.

[efiop]

@dtrifiro Could you please submit a docs PR for https://dvc.org/doc/command-reference/run ?

[skshetry]

Looks good. It'd be great if you could move these logics to somewhere else, like dvc/repo/params/utils.py as load_params(path_info), but I'm fine with it for now though.

[dtrifiro]

@dtrifiro Could you please submit a docs PR for https://dvc.org/doc/command-reference/run ?

@efiop
treeverse/dvc.org#1612

[efiop]

@dtrifiro Thank you so much! 🙏