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

[FEATURE] Allow computing lambdas on series instead of dataframes #99

Closed
mmuesly opened this issue Jan 2, 2024 · 4 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@mmuesly
Copy link
Contributor

mmuesly commented Jan 2, 2024

Is your feature request related to a problem? Please describe.
We love the implementation for characterizing tetrades in REE patterns on a dataframe. For performance considerations, we require to wrap the implementation into an apply function on a dataframe. In its current form, it is not possible to compute lambdas only on a single series. I suggest to change the implementation to also support the core computation of lambdas on series objects. As a consequence, it can be used inside an apply function and offers easy parallelization in combination with the pandarallel package.

Moreover, I would love to stop the pyrochem.REE to drop PM if it is already in the dataset. Preparing plots gets more complicated if the already added column is dropped by this column.

I propose these changes along with a PR that implements them.

@morganjwilliams
Copy link
Owner

Hey @mmuesly, thanks for opening this issue and sending in the pull request. I'll take a look at the PR today.

I'd only realized in passing lately that some of the accessors don't work for series, so it would be good to fix this. I'm also curious what data you're working with where Pm is relevant, but I think we can work something out there too - I might just need to modify some of the other functions around e.g. synthetic and testing data.

@mmuesly
Copy link
Contributor Author

mmuesly commented Jan 12, 2024

We are working with REE patterns and injecting Pm before using apply on the dataframe that runs pyrochem.REE inside a lambda method on the series makes it easier to setup the plots. Otherwise I agree, that Pm will not be relevant in the data. As said, it is written in a way that it will not fail, if Pm is not in the data.

@morganjwilliams
Copy link
Owner

Hey @mmuesly, new version is out with your PR incorporated (thanks!), let me know if you have any residual or new issues with Pm, .apply(...) and/or working with pandarallel, but I'll close this one for now.

@mmuesly
Copy link
Contributor Author

mmuesly commented Feb 26, 2024

Thank you! I will give it a try and get back if there is any new problem. Otherwise, assume it is working lovely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants