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

Rewrite of tokenizer and introduction of object-based test cases #208

Merged
merged 33 commits into from
Oct 29, 2024

Conversation

VisLab
Copy link
Member

@VisLab VisLab commented Oct 25, 2024

This PR represents a rewrite of the tokenizer, which is the syntax parser for hedstrings. The tokenizer now catches errors from bad slashes and blanks in addition to errors caught in the previous parser. An object based data framework for the test cases was introduced to make it easy to add testcases for the tokenizer.

A start was also made on converting the bids.spec tests to the object framework for clearer description of what was being tested and for possible porting to the Python validator for test cases.

@VisLab VisLab requested a review from happy5214 October 25, 2024 20:53
@VisLab VisLab added enhancement New feature or request quality Code quality, not must-fix labels Oct 25, 2024
@VisLab VisLab added bids BIDS integration tests Issues related to testcases validation Tag validation issues parsing String parsing labels Oct 25, 2024
@VisLab
Copy link
Member Author

VisLab commented Oct 26, 2024

@happy5214 I have updated and moved the converter tests to the tests directory. I am going to go ahead and merge to the develop branch. I also corrected a bug in the tokenizer.

In the next PR, I am going to convert some more of the bids.spec tests to bidsTests.spec and make a first pass at converting the events.spec to the separated test format. Eventually, we may be able to pull out some of the common functions to a test utilities, but I don't think we should worry about that yet.

@VisLab
Copy link
Member Author

VisLab commented Oct 27, 2024

@happy5214 Could you please review --- I think this should be merged with develop and soon with master.

Summary:

  1. The main changes are organizational. I started to convert the existing test to tests based on separate datafiles that could easily be ported to use as tests for the python tools. This is a big job and will take some time. The datafiles are now in tests/testData
  2. There were two converters.js files. I renamed one of them to be tagConverter.js and moved it to the parser directory.
  3. I moved all of the .spec.js files that were scattered in the code to the tests subdirectory.
  4. I corrected a bug in the tokenizer -- it wasn't properly detecting slash after open parenthesis.
  5. I corrected a bug in the column splicer --- it was crashing when {HED} was used as a splice but there was not HED column in the tsv.file.

@VisLab
Copy link
Member Author

VisLab commented Oct 28, 2024

Revised the PR to not treat a missing HED column in tsv as an error when {HED} is used.

@happy5214 happy5214 merged commit 9561c87 into hed-standard:develop Oct 29, 2024
3 of 4 checks passed
@happy5214
Copy link
Member

We walked through these changes during a Zoom meeting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bids BIDS integration enhancement New feature or request parsing String parsing quality Code quality, not must-fix tests Issues related to testcases validation Tag validation issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants