Skip to content

Conversation

@PProfizi
Copy link
Contributor

@PProfizi PProfizi commented Jan 24, 2024

Also fix some typehint and some imports.

@PProfizi PProfizi added the bug Something isn't working label Jan 24, 2024
@PProfizi PProfizi added this to the v0.11.0 milestone Jan 24, 2024
@PProfizi PProfizi self-assigned this Jan 24, 2024
@PProfizi PProfizi changed the title Fixes to custom plugins Fix allow GenericDataContainer as output of CustomOperator Jan 24, 2024
@codecov
Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (3972d58) 86.09% compared to head (36bdaa2) 87.98%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1376      +/-   ##
==========================================
+ Coverage   86.09%   87.98%   +1.88%     
==========================================
  Files          81       81              
  Lines        9417     9477      +60     
==========================================
+ Hits         8108     8338     +230     
+ Misses       1309     1139     -170     

custom_type_field.CustomTypeField,
external_operator_api.external_operator_put_out_custom_type_field,
),
(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PProfizi is it tested somewhere else?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbellot000 CodeCov says yes, which is weird because it is not tested explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbellot000 BTW why do we have a type_to_output specific to the custom operators?

Copy link
Contributor Author

@PProfizi PProfizi Jan 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbellot000 I just realized that to be be able to test this (and for this to even work), the changes need to be propagated to dpf-site.zip.
So either I add the capability to update the dpf-site.zip based on the current venv as we discussed previously, and use it at the start of test_python_plugins.py,
or we have to merge it first, then wait for a server sync loop to put it in dpf-site.zip and only then add the tests.
I'd tend to go towards the first proposal, which means I have to code and merge a PR with the dpf-site.zip update feature before this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #1379

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see then, you can maybe add the test ad mark it as skip this way we can remove the skip later?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbellot000 BTW why do we have a type_to_output specific to the custom operators?

I don't understand, where would you put "external_operator_put_out_generic_data_container" instead?

@cbellot000 cbellot000 self-requested a review January 25, 2024 08:04
@PProfizi PProfizi changed the title Fix allow GenericDataContainer as output of CustomOperator Allow GenericDataContainer as input/output of a CustomOperator Jan 25, 2024
@PProfizi PProfizi changed the title Allow GenericDataContainer as input/output of a CustomOperator Allow GenericDataContainer as input/output of a CustomOperator and fix dpf-site.zip update Jan 26, 2024
Signed-off-by: paul.profizi <[email protected]>
@PProfizi PProfizi merged commit 6b73f99 into master Jan 29, 2024
@PProfizi PProfizi deleted the fix/custom_plugin_fixes branch January 29, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants