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

Use same function name with Julia's standard library TOML.jl #145

Open
Paalon opened this issue Jun 4, 2024 · 10 comments
Open

Use same function name with Julia's standard library TOML.jl #145

Paalon opened this issue Jun 4, 2024 · 10 comments

Comments

@Paalon
Copy link
Contributor

Paalon commented Jun 4, 2024

YAML.jl uses different function names but we should use same function names with TOML.jl (and JSON.jl) because most people are accustomed to its names. Also we should not use underscores for function names because Julia don't use them in conventions.

Names of functions and macros are in lower case, without underscores.

  • load, load_allparse, tryparse
  • load_file, load_all_fileparsefile, tryparsefile
  • writeprint

I have no idea about how to treat all.

@kescobo
Copy link
Collaborator

kescobo commented Jun 5, 2024

Not sure I agree with all of these - the semantics of print and write are different for example - if the API of YAML.write are more like that of print, then fine, but other packages CSV.jl comes to mind use write.

This package predates a lot of other conventions, and in general I'm in favor of unifying the API with what is now standard. Would probably want to have a patch release that adds deprecation warnings, and then a separate release where things are changed.

@Paalon
Copy link
Contributor Author

Paalon commented Jun 13, 2024

I propose:

  • parseonly for parsing the only single document in a YAML file, if the YAML file consists of 0 or 2 or more than 2 documents (n == 0 || 2 ≤ n), throw an error. This is the best API for users who want to use YAML as an alternative of JSON or TOML.
  • parsefirst for parsing the first document from 1 or more documents in a YAML file (= load in v0.4.10), if the YAML file consists of 0 documents (n == 0), throw an error.
  • parse for parsing all documents in a YAML file (=load_all in v0.4.10)

Users should be aware of that a YAML file represents 0 or more YAML documents in order that they do not cause possible misuse and errors.

This makes it clear for users that the current behavior that YAML.load("") gives an error (#129 and #143) is not a bug.

@kescobo
Copy link
Collaborator

kescobo commented Jun 14, 2024

This sounds ok to me. We could even make const load = parsefirst and add a deprecation warning before removing it.

@Paalon
Copy link
Contributor Author

Paalon commented Jun 16, 2024

The deprecation code will be:

# YAML.jl v0.5
@deprecate load parsefirst
@deprecate load_all parse
@deprecate load_file parsefirstfile
@deprecate load_all_file parsefile

@Paalon
Copy link
Contributor Author

Paalon commented Jun 16, 2024

YAML spec uses the words load and dump for converting a YAML to a native data and the reverse process.

@Paalon
Copy link
Contributor Author

Paalon commented Jun 16, 2024

If the deprecation is completed, I think it may be good to prepare load and dump for aliases of reading and writing.

@Paalon
Copy link
Contributor Author

Paalon commented Jun 16, 2024

Julia base exports the name parse so we need to replace existing parse to Base.parse to define parse of the module YAML.

@kescobo
Copy link
Collaborator

kescobo commented Jun 18, 2024

Thinking about it again, the semantics of Base.parse are quite specific, and I think it might be a little bit weird to add YAML.parse* that does something different. Same for tryparse. I know I'm reversing course here. @GunnarFarneback what do you think?

@Paalon
Copy link
Contributor Author

Paalon commented Jun 18, 2024

It's the most important interface, it will be good to gather broad opinions.

@GunnarFarneback
Copy link
Contributor

On the whole I think it's better to follow YAML terminology than trying to match some other markup language implementation, regardless whether that's an stdlib.

Even if YAML allows multiple documents in a file or stream, single-document files are by far the more common case and should be easy to use. The proposal to change load_file to parsefirstfile would go from an easily read and understood name to something that's nearly unreadable and much more annoying to write. Although I can agree that there's a certain logic to the proposed function names I value user friendliness much higher in the public API.

So well, I think the current names for the reading functions are basically fine, and better than the alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants