Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Templatereplacer: fix files input namespace + add tests #4348

Merged
merged 1 commit into from
Sep 4, 2020

Conversation

ramirezfranciscof
Copy link
Member

The files input namespace of the TemplatereplacerCalculation was not dynamic (so it was basically an unusable namespace with no inputs in it). This is a quick fix for that. Additionally, I have added a test for all of its basic functionalities and a test specifically for said namespace.

@sphuber
Copy link
Contributor

sphuber commented Sep 2, 2020

@ramirezfranciscof I think you may have used an incorrect branch because this seems to include two unrelated commits.

@codecov
Copy link

codecov bot commented Sep 3, 2020

Codecov Report

Merging #4348 into develop will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4348      +/-   ##
===========================================
+ Coverage    79.15%   79.28%   +0.14%     
===========================================
  Files          468      468              
  Lines        34620    34616       -4     
===========================================
+ Hits         27400    27443      +43     
+ Misses        7220     7173      -47     
Flag Coverage Δ
#django 72.90% <100.00%> (+0.14%) ⬆️
#sqlalchemy 72.09% <100.00%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiida/calculations/templatereplacer.py 83.96% <100.00%> (+58.03%) ⬆️
aiida/engine/daemon/runner.py 79.32% <0.00%> (-3.44%) ⬇️
aiida/orm/nodes/data/dict.py 84.22% <0.00%> (-0.78%) ⬇️
aiida/orm/utils/managers.py 91.81% <0.00%> (-0.26%) ⬇️
aiida/transports/plugins/local.py 81.54% <0.00%> (+0.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f4eb96...78b73dc. Read the comment docs.

@ramirezfranciscof
Copy link
Member Author

@sphuber There, I fixed that, rebased and all test pass. Thanks for the heads up, I promise I will eventually make a good PR from the first push (⌒_⌒;)

Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Thanks @ramirezfranciscof just a minor simplification

tests/calculations/test_templatereplacer.py Outdated Show resolved Hide resolved
tests/calculations/test_templatereplacer.py Outdated Show resolved Hide resolved
@sphuber
Copy link
Contributor

sphuber commented Sep 4, 2020

Sorry @ramirezfranciscof that should have been io.BytesIO(b'some_string'). I will correct it. Anyway need to rebase it

The `files` namespace is supposed to accept any `SinglefileData` or
`RemoteData`, but it was not marked dynamic explicitly. In that case,
only explicitly defined ports are excepted, which is not what is
intended here.
@ramirezfranciscof
Copy link
Member Author

Ah, no problem, I didn't notice either! For me its ready to merge.

@sphuber sphuber merged commit 7c31bc2 into aiidateam:develop Sep 4, 2020
@sphuber sphuber deleted the hotfix_temprep branch September 4, 2020 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants