dev-cafe / parselglossy

Generic input parsing library, speaking in tongues.
https://parselglossy.readthedocs.io
MIT License
7 stars 2 forks source link

Detects cyclic dependencies of defaults in the template #96

Closed bast closed 4 years ago

bast commented 4 years ago

Notes:

codecov[bot] commented 4 years ago

Codecov Report

Merging #96 into master will increase coverage by 0.23%. The diff coverage is 100.00%.

robertodr commented 4 years ago
bast commented 4 years ago

Later we can use that graph (dependencies_hashable) to sort the dependencies to set defaults. We also probably need to improve the error message but you will see it when you run it on a cyclic example.

robertodr commented 4 years ago

We have some tech for collecting and collating errors from the tree, so that can maybe be reused.

robertodr commented 4 years ago

This is ready to go. Errors are reported nicely:

Errors occurred when checking the template:
- At user['another_section']['some_number']:
  keyword depends cyclically on keyword user['another_section']['another_number']
- At user['some_section']['some_number']:
  keyword depends cyclically on keyword user['some_section']['another_number']

I had to use the symple_cycles function of NetworkX to catch multiple cyclic dependencies. I'll fix the testing errors (somehow they don't happen locally :thinking:) Once this is in we can think of how to reorder the graph.

bast commented 4 years ago

Great work!

robertodr commented 4 years ago

Tests are failing because the error can be reported in many different forms:

Errors occurred when checking the template:
- At user['another_section']['another_number']:
  Keyword depends cyclically on keyword user['another_section']['some_number']
- At user['some_section']['another_number']:
  Keyword depends cyclically on keyword user['some_section']['some_number']

or

Errors occurred when checking the template:
- At user['another_section']['some_number']:
  Keyword depends cyclically on keyword user['another_section']['another_number']
- At user['some_section']['another_number']:
  Keyword depends cyclically on keyword user['some_section']['some_number']

and variations. The cycle finder does not preserve ordering, it seems. I'll extend the regex...

bast commented 4 years ago

In the longer term I will try to simplify the testing. It looked a bit too abstract to me but I also see the point of not repeating too much but I think it can be simplified. But I did not manage to work on it yet.

robertodr commented 4 years ago

In the longer term I will try to simplify the testing. It looked a bit too abstract to me but I also see the point of not repeating too much but I think it can be simplified. But I did not manage to work on it yet.

Yes, I agree that it is too complicated. It takes me a bit to get back into it because the tests are heavily parameterized to avoid repetition and boilerplate. I cannot see an easy way to have the cake and eat it too :(

robertodr commented 4 years ago

If this is all green, you can un-draft and merge (I'd suggest squash-merge)

robertodr commented 4 years ago

:green_apple:

bast commented 4 years ago

You need to approve and squash-merge since I was formally the submitter. Thanks for the work!

robertodr commented 4 years ago

Thanks for prototyping it!