Prototyping reflection of external libraries #1132

epignatelli · 2019-08-08T10:36:52Z

There is no actual code-wise dependency.
Although, to test the prototype, you need to switch to the correspondent (identically named) branches in the BHoM_UI and the Grasshopper_Toolkit, and this pr from the Numpy_Toolkit: BHoM/Numpy_Toolkit#2

Issues addressed by this PR

Closes #1131
This pr is adding a way to reflect assemblies that are not BHoM-compliant, but can be useful in the BHoM workflow.
I have put together a quick wiki for a more exhaustive explanation of the principles I followed, and how to implement another in the future: https://github.com/BHoM/documentation/wiki/The-%60External%60-class

The other main change is this pr is the migration of some of the methods from the UI, that was not really in the best place.
I will put specific comments on the diffs to explain them, I think it is easier to explain and review without missing something.

Test files

The test comes with the PR on the Numpy_Toolkit: BHoM/Numpy_Toolkit#2

Changelog

Migrated GroupMethodsByName from the UI to the Data_Engine and renamed it to GroupByName, since it is a generic method
Refactored the UI method CompileFunction to Engine.Reflection.Convert.ToFunc in order to transform any delegate to a Func<object[], object>
Provide a way to expose instance methods, and any method that returns void
Migrate Engine.Serialiser.Create.MethodBase to Engine.Reflection.Create.MethodBase. The List<string> paramTypes now is the list of name of the types, rather than their json representation
Added a way to create MethodBases via a List<Type> as well, for robustness and performance
Migrate Engine.Serialiser.Create.MethodInfo to Engine.Reflection.Create.MethodInfo
Add the IsExposed method to centralise the check of which methods to expose to the UIs.
Add Engine.Reflection.Query.ExternalMethodList as a way to collect methods from external libraries
Add ability to collect methods declared in nested classes

…nder one delegate type `Func<object[], object>`

epignatelli · 2019-08-16T13:23:12Z

Reflection_Engine/Create/MethodBase.cs

+                return method;
+
+            // So, let's try the other overload
+            return MethodBase(type, methodName, paramTypes.Select(x => x.Name).ToList());


Uses the old overload in case the new fails when dealing with generic types (I believe around 10% of the cases, maybe less)

epignatelli · 2019-08-16T13:23:36Z

Reflection_Engine/Create/MethodInfo.cs

+        /**** Public Methods              ****/
+        /*************************************/
+
+        public static MethodInfo MethodInfo(this Type declaringType, string methodName, List<Type> paramTypes)


Migration from the BHoM_UI

epignatelli · 2019-08-16T13:25:07Z

Reflection_Engine/Modify/MakeStatic.cs

+        [Description("Returns the specific type from a type that contains generic parameters")]
+        [Input("method", "The generic type")]
+        [Output("type", "The specific type constructed from the generic one")]
+        public static MethodInfo MakeStatic(this MethodInfo method)


This method is not correct, because I was not really able to convert an instance method into a static method.. Thus, it is not really used. We may consider removing it altogether.

Yes, I suppose you now have figured out why it cannot work after your work on the UI side.
I would suggest to remove this then. No point on leaving public methods that don't work in the code.

epignatelli · 2019-08-16T13:26:04Z

Reflection_Engine/Query/IsExposed.cs

+        /**** Public Methods - Interfaces               ****/
+        /***************************************************/
+
+        public static bool IIsExposed(this object obj)


This one centralises the check of what to reflect to the UIs.
The current approach spreads around a combination of IsNotImplemented and IsDeprecated.

epignatelli · 2019-08-16T13:28:54Z

Reflection_Engine/Query/MethodList.cs

@@ -59,6 +59,18 @@ public static List<MethodBase> AllMethodList()
            return m_AllMethodList;
        }

+        /***************************************************/
+
+        public static List<MethodBase> ExternalMethodList()


This one collects the methods that come from external libraries.

I have chosen to store the methods into a standalone field to avoid contaminating the other ones.
The reason is that the namespace of the methods the field stores do not start with BH.

epignatelli · 2019-08-16T13:29:56Z

Reflection_Engine/Query/NestedMethods.cs

+        /**** Public Methods                            ****/
+        /***************************************************/
+
+        public static List<MethodInfo> NestedMethods(this Type type)


This add the feature to retrieve methods declared in nested classes

epignatelli · 2019-08-16T13:31:02Z

Serialiser_Engine/Objects/BsonSerializers/MethodBaseSerializer.cs

@@ -88,18 +88,20 @@ public override MethodBase Deserialize(BsonDeserializationContext context, BsonD
            bsonReader.ReadName();
            string methodName = bsonReader.ReadString();

-            List<string> paramTypes = new List<string>();
+            List<string> paramTypesJson = new List<string>();


this improves the clarity of the variable: it's not the name of the type, it is its json representation

epignatelli · 2019-08-16T13:32:54Z

Serialiser_Engine/Objects/BsonSerializers/MethodBaseSerializer.cs

            bsonReader.ReadEndArray();

            context.Reader.ReadEndDocument();

            try
            {
-                MethodBase method = Create.MethodBase((Type)Convert.FromJson(typeName), methodName, paramTypes);
+                List<Type> types = paramTypesJson.Select(x => Convert.FromJson(x)).Cast<Type>().ToList();
+                MethodBase method = Create.MethodBase((Type)Convert.FromJson(typeName), methodName, types); // type overload


Passing the actual types to the reflection engine. In the current master, Create.MethodBase lies in the Serialisation_Engine exactly because paramTypes is a json string, and not the name of the method.

By deserialising the types before the method call allows us to move the Create.MethodBase in its correct namespase: Engine.Reflection.

epignatelli

I left some comments in the code to help reviewing this pr

adecler

Globally happy with the changes. Well done :-)
Just one thing to change :-)

adecler · 2019-08-27T07:06:16Z

Reflection_Engine/Modify/MakeStatic.cs

+        [Description("Returns the specific type from a type that contains generic parameters")]
+        [Input("method", "The generic type")]
+        [Output("type", "The specific type constructed from the generic one")]
+        public static MethodInfo MakeStatic(this MethodInfo method)


Yes, I suppose you now have figured out why it cannot work after your work on the UI side.
I would suggest to remove this then. No point on leaving public methods that don't work in the code.

epignatelli · 2019-08-27T08:14:56Z

Thanks @adecler - yeah, that method doesn't have reason to exist.
Removed in the last commit.

yes, it can happen, I don't know how

adecler

All look good to me. 👍

initial prototype

2004f56

epignatelli added the type:feature New capability or enhancement label Aug 8, 2019

epignatelli added this to the BHoM 2.4 β MVP milestone Aug 8, 2019

epignatelli requested review from al-fisher and adecler August 8, 2019 10:36

epignatelli self-assigned this Aug 8, 2019

epignatelli changed the title ~~BHoM_Engine: Prototyping reflection of external libraries~~ [WIP] BHoM_Engine: Prototyping reflection of external libraries Aug 8, 2019

Eduardo Pignatelli added 10 commits August 8, 2019 15:33

start migrating ui methods into reflection namespace

9188e17

don't know how to deal with generic methods yet - return null for now

fe70cce

clean tree

63ac9a9

MakeStatic is now WIP

94a65b4

Move Create.MethodBase to the Reflection_Engine

e1565ba

Move Query.GenericTypeContraint to the Reflection_Engine

ddbd5e6

Move Query.ParameterWithConstraints to the Reflection_Engine

598a2b8

Remove serialisation of Delegates

0222033

Fix Deserialisation of MethodBase

d3b8316

Fix association of components with back-end methods that return void

7df300d

al-fisher added the status:WIP PR in progress and still in draft, not ready for formal review label Aug 12, 2019

Eduardo Pignatelli added 11 commits August 13, 2019 10:37

improve readability for parameter in Engine.Reflection.Create.MethodBase

4936892

Rename Compute.Compile to Create.Delegate

4b55f13

replace instances

ee037bc

Rally all cross-cases of isntance method and methods returning void u…

3841f34

…nder one delegate type `Func<object[], object>`

Move method to collected methods in nested types from Numpy_Toolkit

5a16b10

files doesn't require this change

cfbf766

fix nestedmethods aggregation

9fbf089

make extraction of external methods more robust

099556c

external methods should not be in the bhom method list

0acb971

Store methods and constructors in one field - switch to MethodBase

cef75d2

Settle on class and method names

b84f01d

epignatelli commented Aug 16, 2019

View reviewed changes

epignatelli marked this pull request as ready for review August 16, 2019 13:44

epignatelli requested a review from rwemay as a code owner August 16, 2019 13:44

epignatelli removed the status:WIP PR in progress and still in draft, not ready for formal review label Aug 16, 2019

This was referenced Aug 16, 2019

Prototyping reflection of external libraries BHoM/BHoM_UI#120

Merged

Prototyping reflection of external libraries BHoM/Grasshopper_UI#393

Merged

epignatelli changed the title ~~[WIP] BHoM_Engine: Prototyping reflection of external libraries~~ BHoM_Engine: Prototyping reflection of external libraries Aug 19, 2019

Eduardo Pignatelli added 2 commits August 21, 2019 12:17

rename MakeGeneric to MakeFromGeneric to be clearer

4c1ef99

fix MakeFromGeneric file name

266b17c

adecler requested changes Aug 27, 2019

View reviewed changes

removing the Reflection.Modify.MakeStatic method

a84af06

Eduardo Pignatelli added 3 commits August 27, 2019 09:31

pushing the csproj file

6671d60

Remove gatherer methods from the list of external methods

31c59fd

adding check for null namespace

54b498f

yes, it can happen, I don't know how

adecler approved these changes Aug 28, 2019

View reviewed changes

adecler merged commit b835317 into master Aug 28, 2019

adecler deleted the BHoM_Engine-#1131-ReflectingExternalLibraries branch August 28, 2019 02:03

FraserGreenroyd changed the title ~~BHoM_Engine: Prototyping reflection of external libraries~~ Prototyping reflection of external libraries Sep 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prototyping reflection of external libraries #1132

Prototyping reflection of external libraries #1132

epignatelli commented Aug 8, 2019 •

edited

Loading

epignatelli Aug 16, 2019

epignatelli Aug 16, 2019 •

edited

Loading

epignatelli Aug 16, 2019

adecler Aug 27, 2019

epignatelli Aug 16, 2019

epignatelli Aug 16, 2019

epignatelli Aug 16, 2019

epignatelli Aug 16, 2019

epignatelli Aug 16, 2019

epignatelli left a comment

adecler left a comment

adecler Aug 27, 2019

epignatelli commented Aug 27, 2019

adecler left a comment

Prototyping reflection of external libraries #1132

Prototyping reflection of external libraries #1132

Conversation

epignatelli commented Aug 8, 2019 • edited Loading

Issues addressed by this PR

Test files

Changelog

Choose a reason for hiding this comment

epignatelli Aug 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epignatelli left a comment

Choose a reason for hiding this comment

adecler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epignatelli commented Aug 27, 2019

adecler left a comment

Choose a reason for hiding this comment

epignatelli commented Aug 8, 2019 •

edited

Loading

epignatelli Aug 16, 2019 •

edited

Loading