Enumerated ConditionSet

[mjungmath]

We implement a support for enumerated sets of ConditionSet (see #32089).

CC: @mkoeppe @tscrim @trevorkarn

Component: symbolics

Author: Michael Jung

Branch/Commit: b5e4168

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/32315

[mjungmath]

Branch: public/32315_conditionset_enumerated

[sagetrac-git]

Commit: d9b6ddb

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

d9b6ddb

Trac #32315: support enumerated sets

[mjungmath]

comment:3

Feel free to add more examples or make some changes. I removed the pre-processing with __private_classcall__ and shifted it to a factory method.

If you agree with the changes, it's ready for review.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c8062e4

Trac #32315: add new line at the end

[sagetrac-git]

Changed commit from d9b6ddb to c8062e4

[tscrim]

comment:5

Strong -1 on factory functions. It further separates the code from what it works on and now you don't have a natural "isinstance"

[mjungmath]

comment:6

The problem I faced was that __private_classcall__ should delegate to the enumerated sub-class version. But then the whole code in __private_classcall__ should be copied to the sub-class again. This is bad for maintenance.

Do you have a better idea?

[tscrim]

comment:7

If it does need to be used by subclasses, then it should be __classcall__. However, you can do all of thebpreprocessing in the private version, then pass off to the correct subclass. What I am saying is it would be your factory function.

Also, thank you for adding this feature.

[mjungmath]

comment:8

And leave a note that the sub-class, i.e. enumerated version, is not intended to be used by the end-user?

[mjungmath]

comment:9

Otherwise, if __classcall__ delegates to the sub-class and is used by the sub-class again, this leads to an infinite loop...

[tscrim]

comment:10

They are never meant to be called directly, no. Although I don't necessarily see why we need a subclass. I also don't get how you are getting an infinite loop. It is how function inheritance works.

[mjungmath]

comment:11

Replying to @tscrim:

Also, thank you for adding this feature.

Thank you Travis for suggesting this, and your help in general! :)

Replying to @tscrim:

They are never meant to be called directly, no. Although I don't necessarily see why we need a subclass.

A non-enumerated set is not supposed to have an __iter__ method, no? That's why I thought that sub-classing is necessary. Or what would have been your approach?

I also don't get how you are getting an infinite loop. It is how function inheritance works.

Well, __classcall__ of the base-class returns an invokation of the enumerated class. But that invokes the same __classcall__ method, invoking the class constructor again... Or do I have a flaw in my thinking?

[tscrim]

comment:12

Replying to @mjungmath:

They are never meant to be called directly, no. Although I don't necessarily see why we need a subclass.

A non-enumerated set is not supposed to have an __iter__ method, no? That's why I thought that sub-classing is necessary. Or what would have been your approach?

set also has an iterator. It just doesn't promise anything precise about the iteration order.

I also don't get how you are getting an infinite loop. It is how function inheritance works.

Well, __classcall__ of the base-class returns an invokation of the enumerated class. But that invokes the same __classcall__ method, invoking the class constructor again... Or do I have a flaw in my thinking?

You would likely have a __classcall_private__ to handle delegating to which subclass, then the processing of the input is done in __classcall__. Although another option is just a _process_inputs type method.

[mjungmath]

comment:13

I see, then no sub-classing is necessary. That makes this ticket indeed minimal-invasive.

The latter is good to know. Does __classcall_private__ always go over __classcall__?

[tscrim]

comment:14

I am actually sure if a class has both of them implemented what happens. If it is in a base class, then the order is clear I think.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b5e4168

Trac #32315: support iteration and enumerated sets

[sagetrac-git]

Changed commit from c8062e4 to b5e4168

[mjungmath]

comment:16

Alright. Small misunderstanding, huge impact.

This should be fine now.

[mjungmath]

comment:17

Thank you Travis for pointing these things out.

[tscrim]

comment:18

Thanks. LGTM.

[tscrim]

Reviewer: Travis Scrimshaw

[mkoeppe]

comment:19

Thanks for implementing this!

[vbraun]

Changed branch from public/32315_conditionset_enumerated to b5e4168