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.

Sign up for GitHub

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

Jump to bottom

atdpy treats option as nullable #332

Closed
mjambon opened this issue Mar 11, 2023 · 0 comments · Fixed by #333
Closed

atdpy treats option as nullable #332

mjambon opened this issue Mar 11, 2023 · 0 comments · Fixed by #333
Labels
bug target:python Issues related to the Python backend (atdpy)

Comments

@mjambon
Copy link
Collaborator

mjambon commented Mar 11, 2023

atdpy treats the option type as nullable but it should not.

The convention used by ATD is to use "None" or ["Some", 42] to represent int option values in JSON. Python code generated by atdpy instead expects null or 42 as if the type was int nullable rather than int option.

I don't think we need to support the option type at all for Python since Python doesn't have a standard option type. The Optional type known by mypy is in fact equivalent to ATD's nullable, not option.

Instead of emitting an error, we could create an option type for Python but it would probably be awkward. This can be done later if there's some demand for it (I doubt it will be the case).
@mjambon mjambon added bug target:python Issues related to the Python backend (atdpy) labels Mar 11, 2023
@mjambon mjambon mentioned this issue Mar 14, 2023
Merged
3 tasks
mjambon added a commit that referenced this issue Mar 14, 2023
@mjambon
Add mostly-correct support for the option type in Python.
f810db8 
The JSON representation is now compatible with ATD's convention.
The Python representation, however, is still a nullable since Python
doesn't have a standard option type.
See #332
@mjambon mjambon mentioned this issue Mar 14, 2023
Merged
3 tasks
mjambon added a commit that referenced this issue Mar 14, 2023
@mjambon
atdpy: fix JSON representation of the option type (#333)
fc3bcfb 
* Disable incorrect interpretation of 'option' type in atdpy.

* Update changelog

* Add mostly-correct support for the option type in Python.
The JSON representation is now compatible with ATD's convention.
The Python representation, however, is still a nullable since Python
doesn't have a standard option type.
See #332

* Update documentation
@mjambon mjambon mentioned this issue May 12, 2023
Merged
mjambon added a commit to mjambon/opam-repository that referenced this issue May 12, 2023
@mjambon
[new release] atdts, atds, atdpy, atdj, atdgen, atdgen-runtime, atdge…
bbec61b 
…n-codec-runtime and atd (2.12.0)

CHANGES:

* atdgen: Annotate generated code with types to disambiguate OCaml
  classic variants (ahrefs/atd#331)
* atdpy: Support the option type more correctly so that it follows
  ATD's convention for JSON encoding. This allows compatibility with
  JSON produced by other tools of the ATD suite. The Python type,
  however, is still a nullable (`Optional`) to make things simpler for
  Python programmers. This prevents distinguishing `["Some", "None"]`
  from `"None"` which both translate to `None` in Python. (ahrefs/atd#332)
* (BREAKING) atdgen: revert default encoding of int64 values as string (ahrefs/atd#330)
* atdgen: Support `<json repr="string">` for `int` values (ahrefs/atd#330)
* atdpy: Treat default field values as expressions to evaluate each time
  they're assigned to a field. This allows the use of mutable defaults such as
  lists (ahrefs/atd#339)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug target:python Issues related to the Python backend (atdpy)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

atdpy: fix JSON representation of the option type ahrefs/atd
1 participant
@mjambon