Add multilayered inheritance structure to DataConcepts[Collection]

[dekatzenel]

Citrine Python PR

Description

Add multilayered inheritance structure to DataConcepts[Collection]. Allows for more fine-grained control of which classes have access to shared methods/values

PR Type:

• Breaking change (fix or feature that would cause existing functionality to change)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Adherence to team decisions

• I have added tests for 100% coverage
• I have written Numpy-style docstrings for every method and class.
• I have communicated the downstream consequences of the PR to others.

[bfolie]

I think this is an improvement. A few questions.

1. Are you also going to use this PR to fix the filter_by_attribute_bounds issue that instigated the change?
2. Is there a need to have the abstract get_type method in each abstract collection class? Since they implement DataConceptsCollection and the method is abstractly defined there, it seems redundant.
3. Should we create separate classes for specs and runs? On the one hand it doesn't necessarily hurt, and we could always define the two classes and if we don't make use of them that's ok. But I also don't necessarily view specs and runs as being different types of objects, and can't think of a method we'd want to do to one but not the other. Do you have a distinction in mind?

[dekatzenel]

1. Are you also going to use this PR to fix the `filter_by_attribute_bounds` issue that instigated the change?

I was planning to do that as a separate PR for convenience of review. I intended to do it prior to adding another non-template endpoint (which is how I found this issue in the first place)

[dekatzenel]

2. Is there a need to have the abstract `get_type` method in each abstract collection class? Since they implement `DataConceptsCollection` and the method is abstractly defined there, it seems redundant.

It removed a warning in intellij. If we're cool with the warning, I can remove the abstract method. Apparently having these classes extend ABC also fixes the warning, if that sounds better

[dekatzenel]

3. Should we create separate classes for specs and runs? On the one hand it doesn't necessarily hurt, and we could always define the two classes and if we don't make use of them that's ok. But I also don't necessarily view specs and runs as being different types of objects, and can't think of a method we'd want to do to one but not the other. Do you have a distinction in mind?

I do actually - consider MaterialRunCollection.filter_by_spec(), MaterialRunCollection.filter_by_template(), and MaterialSpecCollection.filter_by_template() in a world where these methods are implemented for all runs and specs, respectively.

[bfolie]

It removed a warning in intellij. If we're cool with the warning, I can remove the abstract method. Apparently having these classes extend ABC also fixes the warning, if that sounds better

Yeah I think it makes the most sense to extend ABC