BHoM / BHoM_Adapter

GNU Lesser General Public License v3.0
7 stars 5 forks source link

Add adapter module for fetching dependency objects #305

Closed IsakNaslundBh closed 2 years ago

IsakNaslundBh commented 2 years ago

Issues addressed by this PR

Closes #304

Adds AdapterModule for fetching dependency objects for edgecases where the method in Reflection_Engine can not handle it.

First usecase added for getting Loadcases from LoadCombinations

Test files

Changelog

Additional comments

IsakNaslundBh commented 2 years ago

@BHoMBot check compliance @BHoMBot check required

bhombot-ci[bot] commented 2 years ago
@IsakNaslundBh to confirm, the following checks are now queued: - `code-compliance` - `documentation-compliance` - `project-compliance` - `branch-compliance` - `dataset-compliance` - `copyright-compliance` - `code-compliance` - `documentation-compliance` - `project-compliance` - `core` - `null-handling` - `serialisation` - `installer` - `versioning`
bhombot-ci[bot] commented 2 years ago
The check `code-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 2 years ago
The check `documentation-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 2 years ago
The check `project-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
IsakNaslundBh commented 2 years ago

@BHoMBot check compliance @BHoMBot check required

bhombot-ci[bot] commented 2 years ago
@IsakNaslundBh to confirm, the following checks are now queued: - `code-compliance` - `documentation-compliance` - `project-compliance` - `branch-compliance` - `dataset-compliance` - `copyright-compliance` - `code-compliance` - `documentation-compliance` - `project-compliance` - `core` - `null-handling` - `serialisation` - `installer` - `versioning` There are 10 requests in the queue ahead of you.
alelom commented 2 years ago

The change makes sense to me. However, I'm considering whether we could revise the GetDependencyObjects() signature.

The inner logic effectively makes the adapter parameter optional, as the method proceeds whether it finds any module or not. Therefore I would either:

GetDependencyObjects<T>(IEnumerable<T> objects, List<Type> dependencyTypes, IBHoMAdapter adapter = null)

I just think this would make things clearer; even though one method would probably rarely be used, we still should consider that this method is publicly available as part of an Engine. What do you think @IsakNaslundBh ?

IsakNaslundBh commented 2 years ago

I just think this would make things clearer; even though one method would probably rarely be used, we still should consider that this method is publicly available as part of an Engine. What do you think @IsakNaslundBh ?

Happy to change to that!

Also, as this can not go in until next milestone, would not mind to have a quick brainstorm call with you about this in general, and maybe iron out a few things here before merge anyway :)

alelom commented 2 years ago

would not mind to have a quick brainstorm call with you about this in general, and maybe iron out a few things here before merge anyway

sounds great!

IsakNaslundBh commented 2 years ago

@BHoMBot check core

IsakNaslundBh commented 2 years ago

@JosefTaylor @alelom I had to rebase and forcepush an update to this PR due to the framework update PR https://github.com/BHoM/BHoM_Adapter/pull/308 that was merged last week. To be able to retest this, please ensure to delete the branch locally and pull it again.

IsakNaslundBh commented 2 years ago

@BHoMBot check compliance @BHoMBot check required

bhombot-ci[bot] commented 2 years ago
@IsakNaslundBh to confirm, the following checks are now queued: - `code-compliance` - `documentation-compliance` - `project-compliance` - `branch-compliance` - `dataset-compliance` - `copyright-compliance` - `code-compliance` - `documentation-compliance` - `project-compliance` - `core` - `null-handling` - `serialisation` - `installer` - `versioning`
bhombot-ci[bot] commented 2 years ago
The check `code-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 2 years ago
The check `documentation-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
bhombot-ci[bot] commented 2 years ago
The check `project-compliance` has already been run previously and recorded as a successful check. This check has not been run again at this time.
IsakNaslundBh commented 2 years ago

@BHoMBot check ready-to-merge

bhombot-ci[bot] commented 2 years ago
@IsakNaslundBh to confirm, the following checks are now queued: - `ready-to-merge` There are 12 requests in the queue ahead of you.