Elsewhere the type is specified as set. However, as this is not a subclass of set, that would be incorrect. Also, later when we remove this, then we will have a set. Perhaps this class should properly reflect that now. Unfortunately, there are no specialized container datatypes for sets in collections. And sets are not a Sequence Type either. Not really sure how best to address this.

My feeling was that it was OK to lie about the type, so that users actually get type warnings when they use it as a list instead of a set. That's one more (very effective) way of communicating the change. And since we implement all set functionality, the lie is not too big 🙂

We could do that, but some users run type validation on their code and this could result in bug reports. Either we need to fully document our reasoning for the inconsistency in the code comments or we need to make a change to the code.

Users running type checkers on their code is exactly what I expect, yes 😄 So, my opinion is, yes, lets keep lying about the type, and document that prominently (both with code comments and release/changelog notes).

Type correctness would otherwise mean that:

• we type the variables as _BlockLevelElements
• which in turn means we'd have to document this class properly
• which in turn (probably) means we'd have to make it public

(also we'd likely want to use Self from typing_extensions as return annotation of methods retuning self, which means an additional dependency depending on the Python version)

I know it's far from being exhaustive but a search on GitHub returns only one result for use of md.block_level_elements (apart from @facelessuser's pymdownx which creates a new set from it, and discarding forks and venvs), and it's using remove, which is both a list and set method. That's IMO a strong indicator that md.block_level_elements has very few uses in the wild, and so immediately advertising its type as set should have a very small impact. I could be wrong though 🤷

It's not just type validators that I'm thinking about here. What about someone who is using isinstance (or similar) on the object in their code? They could reasonably expect isinstance(set) to return True, which is does not. For that matter existing code could reasonable expect isisntance(list) to return True. My objection is that neither return True. However, if _BlockLevelElements was a subclass of something, then it could return True for one of either set or list. That's why I pointed out above that set is not a Sequence Type. I was hoping it was and then _BlockLevelElements could just subclass the relevant baseclass and return True for both. I think we need to return True for at least one. The question is: which one? Using set is a breaking change but what it will end up being when the transition is complete. Using list avoids the breaking change, but it not forward looking. And I don't think there is any way to raise a deprecation warning if a user calls isinstance on the objection.

My point is that I think _BlockLevelElements needs to be either a set or a list at a minimum. Ideally it would be both, but that is probably not sensible. Any solution is going to be a compromise. I'm saying that having _BlockLevelElements not be either a set or a list is not the compromise I want to make.

OK __instancecheck__ works the other way unfortunately, it's not meant for this use-case. Inheriting from both set and list is not possible: CPython doesn't allow it. Inheriting from set breaks a lot of tests (121 of them), not sure exactly why. Inheriting from list works, but as you pointed, isinstance(ref, set) will return False.

Inheriting from both set and list is not possible: CPython doesn't allow it.

That is what I thought. Not sure why, but I had a vague recollection that that was the case.

Inheriting from set breaks a lot of tests (121 of them), not sure exactly why.

Do these tests fail if we use set directly rather than out custom class? If so, we really need to track this down. We don't want to encounter this issue for the first time in a future step of the deprecation...

That is if we ever make the deprecation. I'm thinking that this may be a blocker for this change moving forward. Can you provide some clear date on the improvements this one change makes?

Do these tests fail if we use set directly rather than out custom class? If so, we really need to track this down. We don't want to encounter this issue for the first time in a future step of the deprecation...

Tests pass when block level elements variable are sets. I ran a debugging session and it turns out set(md.block_level_elements) returns an empty set for some reason (while list(md.block_level_elements) returns a valid list with all elements). I think it's because by inheriting from set, _BlockLevelElements indeed becomes a set, but an empty one, since we store elements in self._set (and self._list). CPython probably then takes a shortcut and doesn't use __iter__. I believe we can solve this by getting rid of self._set and calling parent methods instead of self._set.METHOD.

That is if we ever make the deprecation. I'm thinking that this may be a blocker for this change moving forward. Can you provide some clear date on the improvements this one change makes?

Yep, totally understandable, 700 lines for changing a variable from list to set is not the most elegant backward compatible change. I don't have clear data, but I can tell you this:

• without the post-processor change, the list->set would have been very important
• but now that the post-processor is much more efficient, the list->set change brings a very, very tiny perf gain (at least in the context of the raw HTML post-processor)

To be honest I wouldn't mind if we dropped the list->set change entirely, to make this PR easier to merge. We can always revisit later.

• but now that the post-processor is much more efficient, the list->set change brings a very, very tiny perf gain (at least in the context of the raw HTML post-processor)

That is pretty significant.

To be honest I wouldn't mind if we dropped the list->set change entirely, to make this PR easier to merge. We can always revisit later.

I agree. Let's do that.

I squashed all fixups, and added a revert commit, so that we keep a trace of it in this PR (while you just have to squash when merging to get rid of both).