Changes to add named fields and some other improvements #67
Replies: 5 comments 18 replies
-
|
Hi @tombolano, Thanks for the feedback and sorry for the delay! I'm definitely very interested by everything you did, but unfortunately my free time has been scarce lately ... Anyway, I certainly hope that we can integrate most of what you did. I'll try to have at least a quick look at your code next week. Just one question at this point : I was also interested in the constructor keyword arguments & named attributes at one stage (I do agree that it's nice to have!) and kinda lost interest when I saw that this information was absent from the Haskell types definition (and I didn't want to settle with a manual naming system that would become obsolete with every new version of the Pandoc types). Do you derive automatically the attribute names? Sometimes there is a sensible name to give (for example name Cheers, Sébastien |
Beta Was this translation helpful? Give feedback.
-
|
Hi @boisgera, Thanks for your response. Yes, the field names are mostly derived automatically, this is done by the function First, the algorithm checks if the declaration defines a record type or a product type:
The final step of the algorithm is to check if there are duplicated field names, if there are then the algorithm renames them by appending '0', '1', etc, at the end. In Pandoc types 1.23.1 this happens only for the
the resulting field names for this case are:
For Pandoc types 1.23.1, the complete list of field names generated for each type is the following: For obtaining this list of types and fields I just ran this code in my branch: import pandoc
pandoc.configure(auto=True)
for type_str, type_val in sorted(pandoc.types._types_dict.items()):
if issubclass(type_val, pandoc.types.Constructor):
print(f"{type_str:<20}", type_val._fields, sep="")I think the results are quite nice, in many cases they are the same or very close to the Pandoc Lua API. However, there are some differences because in the Pandoc Lua API the fields are as far as I know named manually, which introduces some incongruences, and in a few cases the Pandoc Lua API adds more fields to the types (i.e., for these cases the structure of the Lua types is not the same as the Haskell types). Here is a comparison table with some examples: Note: the reference for the Pandoc Lua API types is in https://pandoc.org/lua-filters.html#type-image
|
Beta Was this translation helpful? Give feedback.
-
|
Well, this is a very well thought-out piece! Thanks for this very detailled answer, I see where you're coming from and definitely agree with your decision process (infer what can be inferred automatically, clean up the Lua filter naming inconsistencies, etc.) Two details:
I am definitely interested in reviewing this is detail. I can think of three related issues to consider at the moment: Representation as StringsI don't like very much the representation of pandoc items with keyword arguments by default. When >>> import pandoc
>>> text = "Hello world!"
>>> doc = pandoc.read(text)I think that I'd rather have >>> doc
Pandoc(Meta({}), [Para(content=[Str(text='Hello,'), Space(), Str(text='World!')])])instead of >>> doc
Pandoc(meta=Meta(table={}), blocks=[Para(content=[Str(text='Hello,'), Space(), Str(text='World!')])])but this is mostly a matter of taste. I guess that a mechanism like NumPy's printoptions / set_printoptions could allow the user to switch between both representations? (Note that by the same mechanims we could introduce some pretty-printing that would probably alleviate my distaste for named fields.) On a more practical note: the current library tests rely on the examples used in the documentation that use the compact/positional representation. If we were to change the default to named arguments, all tests would failed ATM. Type DiscoverabilityI really, really like that I can forget the details of the pandoc type hierarchy and find this info in my Python console: >>> from pandoc.types import *
>>> Meta
Meta({Text: MetaValue})
>>> Pandoc
Pandoc(Meta, [Block])
>>> AlignCenter
AlignCenter()
>>> Attr
Attr = (Text, [Text], [(Text, Text)])
>>> Cell
Cell(Attr, Alignment, RowSpan, ColSpan, [Block])To use named arguments with the same degree of convienence (or greater), this type representation must be adapted. For example, something like: >>> Pandoc
Pandoc(meta: Meta, blocks: [Block])(should it also be configurable?) Default ValuesNamed constructor arguments open the way for default values. For example, I'd much rather have >>> doc = Pandoc(blocks=blocks)than >>> doc = Pandoc(Meta({}), blocks)I did not think very deeply of it but I guess that at least some of the cases would be no-brainers (for example make every list and every map empty by default?). Your inputs on this are welcome! 🤗 |
Beta Was this translation helpful? Give feedback.
-
|
Many thanks for your response @boisgera, I respond to all your comments below.
Yes, I agree. Actually I noticed the
Yes, map is a good name. I have updated my branch of the code with the previous changes.
Thanks, I was aware that the tests were relying on the positional representation, so yes, I think that indeed a mechanism to switch between representations is needed. For pretty-printing the best option may be to just use the rich library: https://github.com/Textualize/rich. By default this library already pretty-prints dataclasses, and if we want to print the dataclasses without the keywords it only requires to add a very simple I have updated my branch of the code to implement this, the changes are actually quite simple:
To not show the field names by default I also added a call to Here is a code that shows it working: import rich
import pprint
import pandoc
doc = pandoc.read("Lorem ipsum dolor sit amet, consectetur adipiscing elit")
for i in range(3):
print("-"*40)
if i == 0:
print("Not printing fields (the default)")
elif i == 1:
pandoc.types.print_options(show_fields=True)
print("Setting printing fields")
elif i == 2:
pandoc.types.print_options(show_fields=False)
print("Setting not printing fields")
print()
print("print:")
print(doc)
print()
print("pprint:")
pprint.pp(doc)
print()
print("rich:")
rich.print(doc)The output is the following (note that in the terminal the ouput of the rich module is highlighted with colors, which are not shown here):
Yes, this is also important, I have checked and it is also quite easy to implement. I have already made the changes and uploaded them to my branch. The changes are these:
Here is a code that shows it working: import pandoc.types
print(pandoc.types.Attr)
print(pandoc.types.Cell)
print(pandoc.types.Citation)
print(pandoc.types.Meta)
print(pandoc.types.Pandoc)
pandoc.types.print_options(show_type_fields=False)
print()
print(pandoc.types.Attr)
print(pandoc.types.Cell)
print(pandoc.types.Citation)
print(pandoc.types.Meta)
print(pandoc.types.Pandoc)The ouput is the following:
Yes, this would be really nice. The code in my branch was setting None as the default value for all fields: >>> import pandoc.types
>>> pandoc.types.Pandoc()
Pandoc(None, None)which is not that useful. Note that to implement this feature all fields should have a default value, because in Python non-default arguments cannot follow default arguments. For example: @dataclass
class my_class:
a: str = 'a'
b: strwill return this error: And this: def my_func(a = 'a', b):
passwill return this error: I took a look at this and I managed to implement it in my branch. The implementation is not so difficult, this is what I did:
With the previous changes now the example that you proposed works: >>> import pandoc.types
>>> pandoc.types.Pandoc(blocks=[])
Pandoc(Meta({}), [])The following is the full list of Constructor types with their default values: AlignCenter()
AlignDefault()
AlignLeft()
AlignRight()
AuthorInText()
BlockQuote([])
BulletList([])
Caption(None, [])
Cell(('', [], []), AlignDefault(), RowSpan(0), ColSpan(0), [])
Citation('', [], [], AuthorInText(), 0, 0)
Cite([], [])
Code(('', [], []), '')
CodeBlock(('', [], []), '')
ColSpan(0)
ColWidthDefault()
ColWidth_(0.0)
Decimal()
DefaultDelim()
DefaultStyle()
DefinitionList([])
DisplayMath()
Div(('', [], []), [])
DoubleQuote()
Emph([])
Example()
Figure(('', [], []), Caption(None, []), [])
Format('')
Header(0, ('', [], []), [])
HorizontalRule()
Image(('', [], []), [], ('', ''))
InlineMath()
LineBlock([])
LineBreak()
Link(('', [], []), [], ('', ''))
LowerAlpha()
LowerRoman()
Math(DisplayMath(), '')
Meta({})
MetaBlocks([])
MetaBool(False)
MetaInlines([])
MetaList([])
MetaMap({})
MetaString('')
NormalCitation()
Note([])
OneParen()
OrderedList((0, DefaultStyle(), DefaultDelim()), [])
Pandoc(Meta({}), [])
Para([])
Period()
Plain([])
Quoted(SingleQuote(), [])
RawBlock(Format(''), '')
RawInline(Format(''), '')
Row(('', [], []), [])
RowHeadColumns(0)
RowSpan(0)
SingleQuote()
SmallCaps([])
SoftBreak()
Space()
Span(('', [], []), [])
Str('')
Strikeout([])
Strong([])
Subscript([])
Superscript([])
SuppressAuthor()
Table(('', [], []), Caption(None, []), [], TableHead(('', [], []), []), [], TableFoot(('', [], []), []))
TableBody(('', [], []), RowHeadColumns(0), [], [])
TableFoot(('', [], []), [])
TableHead(('', [], []), [])
TwoParens()
Underline([])
UpperAlpha()
UpperRoman()This was the code used to obtain the previous output: import pandoc.types
for type_str, type_val in sorted(pandoc.types._types_dict.items()):
if issubclass(type_val, pandoc.types.Constructor):
print(type_val()) |
Beta Was this translation helpful? Give feedback.
-
|
Very nice. I have not unpacked everything you've done yet (for example I need to read more about rich), but I really like it so far! At this stage, I think that it would be simpler to add you as a collaborator to my repo, so that you can create an experimental branch with your contributions and make changes with minimal friction. Would that be ok with you? |
Beta Was this translation helpful? Give feedback.
-
|
Thanks @boisgera, I have accepted the invitation. I will create an |
Beta Was this translation helpful? Give feedback.
-
|
I think that the name |
Beta Was this translation helpful? Give feedback.
-
|
Hi @tombolano, Areyou still willing to contribute your work as a branch? I'm starting to have a bit of time to work on the project again, I'd like to get to the point where your work can first be contributed as a pure extension (no breaking changes, just some extra features) as a first move. Cheers, Sébastien |
Beta Was this translation helpful? Give feedback.
-
|
Hi @boisgera, Yes, I'm still willing to contribute my work as a branch. I'm sorry for the long delay; I've been very busy with work over the last few weeks. This week I had time and I checked what work was still needed to do, apart from some minor fixes I think the main thing left is documentation. I've done some local work but haven't committed it yet to a new branch. I'll make the commit tomorrow morning. Best regards, |
Beta Was this translation helpful? Give feedback.
-
|
OK great! I totally get the "bandwith" issue, no worries. |
Beta Was this translation helpful? Give feedback.
-
|
Hello @boisgera, I have uploaded my work to the new branch 'experimental', Apart from the work already discussed before, this has the following changes:
However note that there are some things still not documented:
The only breaking thing with respect to the master version are the docstrings of the types, which as I said now print the field names and default values, and I don't know if this could even be considered "breaking". However this can be changed to be exactly as in master by running Best regards, |
Beta Was this translation helpful? Give feedback.
-
|
Hello @tombolano & everyone, I'd like to discuss again the names selected for the constructor arguments in the experimental branch of the Pandoc Python Library. I'd like these names to be derived as simply as possible from our short type definitions (or their original definitions in I consider that many of your derivation tricks are smart and should feel "obvious" to the Python programmer and therefore consistent with the above principle:
But here are some example of what I perceive as deviations:
Now, here is the silver lining: some of these issues could be resolved if pandoc (Haskell) could introduce some additional type aliases. For example today , pandoc defines the caption type type ShortCaption = [Inline]
data Caption = Caption (Maybe ShortCaption) [Block]instead of data Caption = Caption (Maybe [Inline]) [Block]I think that this approach is better for pandoc and a huge facilitator for our project, since there is more information embedded into the original type definitions. Therefore I'd like to propose the following tentative plan (feedback welcome!):
Let me throw here some suggestions of additional type aliases for pandoc:
|
Beta Was this translation helpful? Give feedback.
-
|
Hello @boisgera, I answer below
Yes, technically would be best that this semantic information was present in the types definitions. I added those deviations because my idea was to have an API similar to the Pandoc Lua API (https://pandoc.org/lua-filters.html#lua-type-reference), and for example the naming of the
Yes, I agree, I think that from a technical point of view is best to stick only with the information in the type definitions. For reference I made the following table of the field names that we generate now in the experimental branch, and the field names that we would obtain if we do not consider any special cases (I ommited the types without fields from the table). Note that some types in the table still have a
Honestly, I think that the field names without special cases are also good in most cases, for example for me
Yes, these extra type aliases would be great. I was also thinking of data TableBody = TableBody !Attr !RowHeadColumns ![Row] ![Row]Which has 2 type TableBodyHead = [Row]
data TableBody = TableBody !Attr !RowHeadColumns !TableBodyHead ![Row]
I think that |
Beta Was this translation helpful? Give feedback.
-
|
Hi Tomás!
👍
100% agree. Here I think that the issue is that there is actually an underlying Haskell pattern (the use of
Agreed on the principle. But do you have a good systematic idea to name a variable of type And finally, agreed that the arguments of |
Beta Was this translation helpful? Give feedback.
-
I've documented here where the Can we come up with something that would work for every
|
Beta Was this translation helpful? Give feedback.
-
|
Hello @boisgera
I was thinking on
I think that the best options are any of
Thanks a lot for explaining the
I personally like both the diff --git a/src/pandoc/types/__init__.py b/src/pandoc/types/__init__.py
index 6806d63..958035f 100644
--- a/src/pandoc/types/__init__.py
+++ b/src/pandoc/types/__init__.py
@@ -68,15 +68,22 @@ def _rename_duplicate_fields(fields: list[str]):
return ret
-def _field_type_name(field_type, field):
+def _field_type_name(field_type, field, recursive=False, prefix=""):
if (
isinstance(field, list)
and field[0] == field_type
and isinstance(field[1], list)
and len(field[1]) == 1
- and isinstance(field[1][0], str)
):
- return field[1][0]
+ if isinstance(field[1][0], str):
+ return _to_snake_case(field[1][0]).lower()
+ elif isinstance(field[1][0], list) and recursive:
+ if name := _field_type_name(field_type, field[1][0], recursive, prefix):
+ return prefix + name
+ else:
+ return None
+ else:
+ return None
else:
return None
@@ -99,6 +106,27 @@ class Field:
type: str | list
+def _get_field_name(type_decl, type_name=None) -> str:
+ if isinstance(type_decl, str):
+ field = type_decl.removeprefix(type_name) if type_name else type_decl
+ field = _to_snake_case(field).lower()
+ elif type_decl[0] == "map":
+ key_field = _get_field_name(type_decl[1][0], type_name).lower()
+ value_field = _get_field_name(type_decl[1][1], type_name).lower()
+ field = f"{key_field}_to_{value_field}"
+ elif field := _field_type_name("maybe", type_decl):
+ if type_name:
+ field = field.removeprefix(type_name.lower()).lstrip("_")
+ elif field := _field_type_name("list", type_decl, True, "list_of_"):
+ if type_name:
+ field = field.removeprefix(type_name.lower()).lstrip("_")
+ field = _to_plural(field)
+ else:
+ field = "content"
+
+ return field
+
+
def _get_data_fields(decl: list) -> list[Field]:
"""
Return for each constructor declaration the name & type of each argument.
@@ -144,47 +172,23 @@ def _get_data_fields(decl: list) -> list[Field]:
if type_def[0] == "map":
for field, type in type_def[1]:
- field = _to_snake_case(field).lower()
- field = field.removeprefix(type_name.lower()).lstrip("_")
- field_names.append(field)
- field_types.append(type)
- else:
- for type in type_def[1]:
- if isinstance(type, str):
- if type == "Int" or type == "Double":
- field = "value"
- else:
- field = _to_snake_case(type.removeprefix(type_name))
- elif field := _field_type_name("maybe", type):
+ if field == "un" + type_name:
+ # detect the haskell pattern un<type name>.
+ # In this case we derive the field name from the type
+ field = _get_field_name(type, type_name)
+ else:
field = _to_snake_case(field).lower()
field = field.removeprefix(type_name.lower()).lstrip("_")
- elif field := _field_type_name("list", type):
- if (
- type_name != "Pandoc"
- and not type_name.startswith("Meta")
- and (field == "Block" or field == "Inline")
- ):
- field = "content"
- else:
- field = _to_snake_case(field).lower()
- field = field.removeprefix(type_name.lower()).lstrip("_")
- field = _to_plural(field)
- else:
- field = "content"
field_names.append(field)
field_types.append(type)
+ else:
+ for type in type_def[1]:
+ field_names.append(_get_field_name(type, type_name))
+ field_types.append(type)
field_names = _rename_duplicate_fields(field_names)
- # Handle special cases
- if type_name == "Meta":
- field_names = _rename_fields(field_names, {"un_meta": "map"})
- elif type_name == "Header":
- field_names = _rename_fields(field_names, {"value": "level"})
- elif type_name == "TableBody":
- field_names = _rename_fields(field_names, {"rows1": "head", "rows2": "body"})
-
return [Field(n, t) for n, t in zip(field_names, field_types)]In these changes:
with these changes the fields names change for the
I you like this I can do a commit with the changes. |
Beta Was this translation helpful? Give feedback.
-
Lists of lists
I like it for every reason but one, and it's a big one : the name is probably misleading. A "list of blocks" is probably most likely interpreted as "list of objects of type I tried to make ChatGPT guess the type of several names and one that works better than At the moment, I'd favor
(I gave a look at the Maps
OK, let's pick Commit
Please do, thanks a lot! 👍 If you're ok with it (we can take more time to discuss & brainstorm if needed), I'll change the |
Beta Was this translation helpful? Give feedback.
-
Ah yes, you are totally right, I actually hadn't thought of that 🤕
Ok, i also think
Ok, great, i will do the commit. Changing the |
Beta Was this translation helpful? Give feedback.
-
|
I have already commited the changes. However, some functions such as |
Beta Was this translation helpful? Give feedback.
-
|
Sure, add docstrings if that makes sense to you (and if you can spare the time). |
Beta Was this translation helpful? Give feedback.
-
Hello, this is a great library, I found it some days ago because I wanted to use the Pandoc AST in Python and this library is exactly what I needed.
One thing that I like about Pandoc is the Lua filters interface, which allows accessing the fields of the elements by their names. Since this library lacked this feature I added it, but before that I also did a general review of the code and changed some other things. My changes are in https://github.com/tombolano/pandoc/tree/experimental. As a summary these are the most relevant changes:
Avoid creating temporary input and output files for communicating with pandoc, use stdin and stdout.
Fixed a bug that caused that the
configureandmake_typesfunctions were called twice when loading thetypesmodule.Modified the
applycode to use a single function for the tree traversal. The typical post order recursive tree traversal function is like this (https://en.wikipedia.org/wiki/Tree_traversal#Post-order_implementation):The current code does the post order tree traversal correctly, but it is split in two functions,
_apply_children, andapply_(insideapply). I found this code a bit confusing and I changed it to just use a single function similar to the one from the example above, called_apply_post_order.Implemented concrete data types as dataclasses. This offers the following features:
Link !Attr ![Inline] !Target, but in the Pandoc Lua filters API it has fieldscontent,target,title, andattr.__init__,__repr__, and__eq__methods are added automatically to the classes.__match_args__variable is created automatically.pprintmodule can be used by default to pretty-print dataclasses, this way we can pretty-print the documents easily.As a drawback, the implementation of the
__getitem__and__setitem__methods now is a little more complicated since we cannot rely directly on a list.Note that the changes do not affect the API of the library, it is still the same, so any previous code should work the same, the only difference is that now because the data types are implemented with dataclasses, when printing a type the field names are also printed. For example, consider this code from the examples in the documentation:
Now with the changes the result is the following:
@boisgera, if you may be interested in some of these changes you may take a look at my commits, I made sure to explain everything in detail in the commit messages. If you are interested I can submit pull requests of the changes, or you may also pick and apply yourself any changes that you want.
Beta Was this translation helpful? Give feedback.
All reactions