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

Added a capacity to give factors to the composite values infiller #72

Merged
merged 12 commits into from Mar 29, 2020

Conversation

Rlamboll
Copy link
Collaborator

Pull request

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Example added (either to an existing notebook or as a new notebook, where applicable)
  • Description in CHANGELOG.rst added

Adding to CHANGELOG.rst

Please add a single line in the changelog notes similar to one of the following:

- (`#XX <https://github.com/znicholls/silicone/pull/XX>`_) Added feature which does something
- (`#XX <https://github.com/znicholls/silicone/pull/XX>`_) Fixed bug identified in (`#YY <https://github.com/znicholls/silicone/issues/YY>`_)

@znicholls
Copy link
Collaborator

Is the idea that creating composite values either takes a list of things to add or you specify factors for every variable? Is the major use case for specifying factors subtraction or am I missing something?

@Rlamboll
Copy link
Collaborator Author

Is the idea that creating composite values either takes a list of things to add or you specify factors for every variable? Is the major use case for specifying factors subtraction or am I missing something?

  1. You can actually mix {str: list} and {str: dict} elements, but I imagine when you are doing complicated things like str: dict you will do it as a different operation each time. The major use case is indeed subtraction, hence the name of the branch, but I thought people might want conversion factors or other things to be done here too.

As you can see I'm having a lot of trouble with the linter, which seems to break things completely when run locally (error: cannot format 05_Multiple_infillers.ipynb: 'charmap' codec can't encode characters in position 20537-20592: character maps to <undefined>, probably a windows encoding bug) and produces unhelpful (though clearly different) error messages on github

@znicholls
Copy link
Collaborator

Nice that's a really clever solution.

I'll format things now and see how I go. I'll turn off notebook formatting in the same PR.

@Rlamboll Rlamboll requested a review from znicholls March 29, 2020 17:01
Copy link
Collaborator

@znicholls znicholls left a comment

Choose a reason for hiding this comment

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

Nice @Rlamboll :D

@Rlamboll Rlamboll merged commit 3e4c2b3 into master Mar 29, 2020
@Rlamboll Rlamboll deleted the subtract_component branch March 29, 2020 21:48
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