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

Extend import functionality #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

jubich
Copy link
Member

@jubich jubich commented Aug 27, 2022

This PR adds the possibility to decide whether an external file should be included via <<< when read by hsd (default) or not (include_file=False). Also, it is now possible to set such imports via Python by adding <<<filename to the dictionary.

It would also be possible (if desired) to add this decision to the import of hsd-files via <<+.

Please let me know if tests should be added.

Copy link
Member

@aradi aradi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A very nice idea! I agree, that it may become quite handy, if the user can set include options explicitly in Python (or keep include options unevaluated when parsing HSD into Python), and I definitely support the idea.

However, I am unsure, whether it is a good idea to do it via strings. We may end up with "<<< some_file" in the output (python->hsd), which is not a valid HSD include statement. Probably, we should use special objects to represent unresolved includes ("interrupts" as the HSD jargon calls them). So on the Python side we could have

{
    some_block: hsd.interrupts.IncludeText("txt_file_name.txt"),
    some_other_block: hsd.interrupts.IncludeHsd("hsd_file_name.hsd")
}

which, when transforming back to HSD could be turned into

some_block {
   <<< "txt_ile_name.txt"
}
some_other_block {
  <<+ "hsd_file_name.hsd"
}

Something along those line would be hopefully more robust and less ambiguous.

src/hsd/dict.py Outdated Show resolved Hide resolved
@jubich jubich force-pushed the fileImport branch 2 times, most recently from baeba37 to 33f9542 Compare March 21, 2023 14:18
@jubich
Copy link
Member Author

jubich commented Mar 21, 2023

I have tried to implement the "interrupts" as I understood them. Please let me know if this meets your expectations or if further changes are needed.

@jubich jubich force-pushed the fileImport branch 2 times, most recently from 9a02b3f to 785ca36 Compare March 30, 2023 15:05
@jubich
Copy link
Member Author

jubich commented Mar 30, 2023

Since the tests failed due to the new @abstractmethod in the HsdEventHandler, maybe it would be a good idea to ask (again) if new tests should be added?

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

Successfully merging this pull request may close these issues.

2 participants