Consolidate separate functions for ions and electrons in plasma parameter calculations #140

RoberTnf · 2017-10-03T16:49:21Z

Closing #106

The checklist of things to do are:

Merge electron_thermal_speed and ion_thermal_speed into thermal_speed
Merge electron_gyrofrequency and ion_gyrofrequency into gyrofrequency
Merge electron_gyroradius and ion_gyroradius into gyroradius
Merge electron_plasma_frequency and ion_plasma_frequency into plasma_frequency
Merge electron_inertial_length and ion_inertial_length into inertial_length
add a parameter method to thermal_speed
I will be updating as I progress with the tasklist.

pep8speaks · 2017-10-03T16:49:25Z

Hello @RoberTnf! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on October 04, 2017 at 12:11 Hours UTC

RoberTnf · 2017-10-03T17:34:49Z

Why do I lack coverage in parts of the code that I didn't touch?

…_frequency

…_length

StanczakDominik

Good stuff overall! By the way, am I right in guessing you're using PyCharm or some other IDE? :D

StanczakDominik · 2017-10-04T09:56:30Z

plasmapy/physics/parameters.py

-    distribution.
+def thermal_speed(T, particle="e"):
+    r"""Returns the most probable speed for an particle within a Maxwellian
+    distributparticle.


What's a distributparticle? ;)

Is a distribution that also accepts electrons.

Ooooh wait, I get it. It's like a distribution, but particle? Does this exhaust the pun limit for physics? 😄

This is an accidental pun, it's what happens when you try to replace the mentions to ion with particle. I will fix it 😄

StanczakDominik · 2017-10-04T09:56:58Z

plasmapy/physics/parameters.py

@@ -380,7 +380,7 @@ def thermal_speed(T, ion="e"):
    >>> from astropy import units as u
    >>> thermal_speed(5*u.eV, 'p')


Do you think you could add an example for the default (electron) value here?

I think that's is covered in line 386: thermal_speed(5*u.eV)

... huh, it is. Sorry, I missed it somehow!

StanczakDominik · 2017-10-04T09:58:01Z

plasmapy/physics/parameters.py

-def ion_plasma_frequency(n_i, ion='p'):
-    r"""Calculates the ion plasma frequency.
+def plasma_frequency(n, particle='e'):
+    r"""Calculates the particle plasma frequency.


"Defaults to the fastest, electron plasma frequency.". How's that sound?

I don't understand what you mean.

I mean we could change the beginning of this docstring like this:

"""Calculates the particle plasma frequency. Defaults to the fastest, electron plasma frequency.

The default behavior of this function seems important enough to mention it straight away :)

Ahh, Ok, sounds fine. Should something similar be done for every other function I modified?

RoberTnf · 2017-10-04T10:03:12Z

I'm using VS Code, why?

StanczakDominik · 2017-10-04T10:09:11Z

Well, your renaming commits come really fast, it's nice to see this kind of proficiency :D

StanczakDominik · 2017-10-04T10:37:16Z

I'm not sure whether it's really necessary for all of them. The plasma frequency sort of stood out to me as I was just reading about it like two days ago. I think we can live without changing it in most of these functions!

…

On 4 October 2017 at 12:34, Roberto Díaz Pérez ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In plasmapy/physics/parameters.py <#140 (comment)>: > }) -def ion_plasma_frequency(n_i, ion='p'): - r"""Calculates the ion plasma frequency. +def plasma_frequency(n, particle='e'): + r"""Calculates the particle plasma frequency. Ahh, Ok, sounds fine. Should something similar be done for every other function I modified? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#140 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKxDL52ZhLpphZj3YjQEkqhAumqaIIdRks5so18ugaJpZM4PsbyS> .

RoberTnf · 2017-10-04T10:46:38Z

I think that's everything, unless I missed something!

StanczakDominik · 2017-10-04T10:47:34Z

Apparently we still have some conflicts on merge, probably because of the doctest thing I merged lately. Could you look into that? :)

RoberTnf · 2017-10-04T10:55:57Z

Ok, I'll try later!

RoberTnf · 2017-10-04T12:00:04Z

So after solving the conflicts I am not able to run pytest, it's trying to run docs/example.py, and obviously it can't. I suppose this is a separate issue but I'm not sure if it's something I did wrong.

StanczakDominik · 2017-10-04T12:01:38Z

Oh yeah, there was a fix in master to this one. Check out these two lines and make sure that's in your code.

In fact, with a proper merge from master this should be working, but git accidents happen all the time.

RoberTnf · 2017-10-04T12:06:42Z

I closed this by accident, sorry, not used to github.

I have those lines, I've pulled master, but I get:

 $ pytest
============================= test session starts ==============================
platform linux -- Python 3.6.2, pytest-3.2.2, py-1.4.34, pluggy-0.4.0
rootdir: /home/roberto/git/PlasmaPy, inifile: pytest.ini
collected 706 items / 2 errors                                                  

==================================== ERRORS ====================================
__________________________ ERROR collecting setup.py ___________________________
/usr/lib/python3.6/distutils/core.py:134: in setup
    ok = dist.parse_command_line()
../../.virtualenvs/plasmapy/lib/python3.6/site-packages/setuptools/dist.py:442: in parse_command_line
    result = _Distribution.parse_command_line(self)
/usr/lib/python3.6/distutils/dist.py:490: in parse_command_line
    raise DistutilsArgError("no commands supplied")
E   distutils.errors.DistutilsArgError: no commands supplied

During handling of the above exception, another exception occurred:
setup.py:38: in <module>
    python_requires='>=3.6',
/usr/lib/python3.6/distutils/core.py:136: in setup
    raise SystemExit(gen_usage(dist.script_name) + "\nerror: %s" % msg)
E   SystemExit: usage: pytest [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
E      or: pytest --help [cmd1 cmd2 ...]
E      or: pytest --help-commands
E      or: pytest cmd --help
E   
E   error: no commands supplied
_______________________ ERROR collecting docs/example.py _______________________
docs/example.py:34: in <module>
    from my_module import my_func, other_func
E   ModuleNotFoundError: No module named 'my_module'
!!!!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!!!!
=========================== 2 error in 1.26 seconds ============================

StanczakDominik · 2017-10-04T12:09:28Z

Happens to everyone :)

This is something we'll have to fix - as a workaround, run it as pytest plasmapy from the root of the repository for now. It should then get the doctest testing config from pytest.ini and avoid docs.

StanczakDominik · 2017-10-04T12:24:09Z

Aaaand congratulations @RoberTnf ! :D

Merged ion_thermal_speed and electron_thermal_speed into thermal_speed

d872f10

fixing PEP-8

61c837f

RoberTnf added 2 commits October 3, 2017 19:52

merged electron_gyrofrequency and ion_gyrofrequency into gyrofrequency

48cd787

merged ion_gyroradius and electron_gyroradius into gyroradius

9553f64

StanczakDominik approved these changes Oct 3, 2017

View reviewed changes

RoberTnf added 6 commits October 4, 2017 11:10

Merged ion_thermal_speed and electron_thermal_speed into thermal_speed

883bfc3

fixing PEP-8

6ef3e9f

merged electron_gyrofrequency and ion_gyrofrequency into gyrofrequency

4eb5dbd

merged ion_gyroradius and electron_gyroradius into gyroradius

3642339

modified the keyword 'ion' to 'species'

adc9146

Merged electron_plasma_frequency and ion_plasma_frequency into plasma…

ad1bc63

…_frequency

StanczakDominik mentioned this pull request Oct 4, 2017

Distribution function #141

Merged

4 tasks

RoberTnf added 2 commits October 4, 2017 11:26

Solve conflicts

d1536f7

Merged electron_inertial_length and ion_inertial_length into inertial…

b30ed4a

…_length

StanczakDominik reviewed Oct 4, 2017

View reviewed changes

Add parameter method to thermal_speed

20fec71

RoberTnf added 2 commits October 4, 2017 12:18

fix PEP8

17a5c91

fixed typo

4432a7c

StanczakDominik mentioned this pull request Oct 4, 2017

Consolidate separate functions for ions and electrons in plasma parameter calculations #106

Closed

6 tasks

StanczakDominik approved these changes Oct 4, 2017

View reviewed changes

Change docstring in plasma_frequency

e78c189

RoberTnf force-pushed the refactor-consolidate-ion-electron branch from 4c49d56 to e78c189 Compare October 4, 2017 10:45

merging from upstream

afe7295

RoberTnf closed this Oct 4, 2017

StanczakDominik reopened this Oct 4, 2017

fixed minor typos in docs

a72ef8f

StanczakDominik merged commit 1b1db10 into PlasmaPy:master Oct 4, 2017

RoberTnf deleted the refactor-consolidate-ion-electron branch October 4, 2017 12:38

namurphy added plasmapy.formulary Related to the plasmapy.formulary subpackage Plasma parameters refactoring ♻️ Improving an implementation without adding new functionality labels Jan 17, 2018

namurphy added this to To Do in Plasma parameters via automation Jan 17, 2018

namurphy added this to the v0.1 milestone Jan 17, 2018

namurphy mentioned this pull request Oct 15, 2018

Update inertial_length #568

Merged

rocco8773 added this to Complete in Formulary Development Jul 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consolidate separate functions for ions and electrons in plasma parameter calculations #140

Consolidate separate functions for ions and electrons in plasma parameter calculations #140

RoberTnf commented Oct 3, 2017 •

edited

Loading

pep8speaks commented Oct 3, 2017 •

edited

Loading

RoberTnf commented Oct 3, 2017

StanczakDominik left a comment

StanczakDominik Oct 4, 2017

RoberTnf Oct 4, 2017

StanczakDominik Oct 4, 2017

RoberTnf Oct 4, 2017

StanczakDominik Oct 4, 2017

RoberTnf Oct 4, 2017

StanczakDominik Oct 4, 2017

StanczakDominik Oct 4, 2017

RoberTnf Oct 4, 2017

StanczakDominik Oct 4, 2017

RoberTnf Oct 4, 2017

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017 via email

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

RoberTnf commented Oct 4, 2017

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017 •

edited

Loading

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

		@@ -380,7 +380,7 @@ def thermal_speed(T, ion="e"):
		>>> from astropy import units as u
		>>> thermal_speed(5*u.eV, 'p')

Consolidate separate functions for ions and electrons in plasma parameter calculations #140

Consolidate separate functions for ions and electrons in plasma parameter calculations #140

Conversation

RoberTnf commented Oct 3, 2017 • edited Loading

pep8speaks commented Oct 3, 2017 • edited Loading

Comment last updated on October 04, 2017 at 12:11 Hours UTC

RoberTnf commented Oct 3, 2017

StanczakDominik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017 via email

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

RoberTnf commented Oct 4, 2017

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017 • edited Loading

RoberTnf commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

StanczakDominik commented Oct 4, 2017

RoberTnf commented Oct 3, 2017 •

edited

Loading

pep8speaks commented Oct 3, 2017 •

edited

Loading

StanczakDominik commented Oct 4, 2017 •

edited

Loading