core: Expose the list of dialects without using xDSLOptMain #1079
Conversation
xdsl/xdsl_opt_main.py
Outdated
| @@ -59,6 +59,56 @@ | |||
| from typing import IO, Dict, Callable, List, Sequence, Type | |||
|
|
|||
|
|
|||
| def get_all_dialects() -> list[Dialect]: | |||
| """Return the list of all available dialects.""" | |||
| # Please keep that list sorted alphabetically. | |||
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1079 +/- ##
==========================================
- Coverage 86.92% 86.90% -0.02%
==========================================
Files 138 138
Lines 20626 20600 -26
Branches 3115 3117 +2
==========================================
- Hits 17929 17903 -26
Misses 2180 2180
Partials 517 517
☔ View full report in Codecov by Sentry. |
|
|
||
|
|
||
| def test_dialects_and_passes(): | ||
| assert len(get_all_dialects()) > 0 |
There was a problem hiding this comment.
sorry, I meant something like this:
| assert len(get_all_dialects()) > 0 | |
| names = [d.__name__ for d in get_all_dialects()] | |
| assert names == sorted(names), "Names of dialects should be in alphabetical order" |
There was a problem hiding this comment.
Do we want to enforce an invariant like that? To me, the dialects are a set of dialects, so it does not even make all that much sense to talk about order 🤔
There was a problem hiding this comment.
Oh no, I don't think we want to have that!
I just want it in the function definition to make it cleaner!
There was a problem hiding this comment.
It would automate this little bit of the process, I don't see the harm in it TBH
There was a problem hiding this comment.
I also don't have a strong opinion, it's just that comments that say "please" in them aren't followed as closely as those that have a matching test to catch transgressors :)
There was a problem hiding this comment.
Yeah I would be against it to be honest.
The harm is that we are adding additional constraints on the API that we don't need, and that may change in the future. So I would prefer not introducing them in the first place.
Also, this check won't work, as dialects are object, not classes, so __name__ won't exist anyway!
There was a problem hiding this comment.
I did not see the comment before, IMO, we should remove that 😅
There was a problem hiding this comment.
@webmiche, do you mean we should remove the comment about keeping them in order?
I added it only because I felt it should be sorted, both for readability and conficts.
But if this cause too much trouble I'm fine removing it
There was a problem hiding this comment.
Hmm not sure. I would remove the comment, but I am fine with leaving it in, as long as we do not actually enforce it.
|
|
||
|
|
||
| def test_dialects_and_passes(): | ||
| assert len(get_all_dialects()) > 0 |
There was a problem hiding this comment.
Do we want to enforce an invariant like that? To me, the dialects are a set of dialects, so it does not even make all that much sense to talk about order 🤔
math-fehr
force-pushed
the
fehr/expose-dialect-list
branch
from
48f3e5a
to
066bcbd
Compare
…ect#1079) This makes it easier for external users to get the list of existing dialects, without having to override xDSLOptMain.
This makes it easier for external users to get the list of existing dialects, without having to override xDSLOptMain.