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

allow units in .sample() #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nstarman
Copy link

Allow initializing Streamspraydf with units and returning unit-ful quantities in .sample().

Signed-off-by: Nathaniel Starkman (@nstarman) nstarkman@protonmail.com

@jobovy
Copy link
Owner

jobovy commented Apr 28, 2021

Can you post an example, e.g., the example from the docs with units on to show that it gives the equivalent result?

@nstarman
Copy link
Author

nstarman commented May 2, 2021

@jobovy
Copy link
Owner

jobovy commented Jul 16, 2021

Hi Nathaniel, just merged this with a change that Yansong made (to allow a stream to be generated in a moving system, like a dwarf galaxy). Could you check that the unit handling still works as expected? I did the merge on GitHub, so am not sure whether it actually worked. I'd like to merge this soon. Thanks!

@nstarman nstarman force-pushed the units_in_sample branch 2 times, most recently from 0dac4f3 to 818bbb3 Compare August 1, 2021 16:35
Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>
@nstarman
Copy link
Author

nstarman commented Aug 1, 2021

I rebased and pushed what I've done. Something I've been trying to figure out for a few days now is why this no longer works.
In https://gist.github.com/nstarman/5a7a1a17ff1759c12d2448846e9a1a9f, the output had units. Now, while safer because of the additions of use_physical=False, there are no units.

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.

2 participants