mgdotdev / librl Goto Github PK

View Code? Open in Web Editor NEW

3.0 2.0 3.0 9.18 MB

A Python library for characterizing Microwave Absorption

License: GNU General Public License v3.0

Python 88.14% TeX 3.94% Makefile 0.24% C++ 7.68%

microwave-absorption umkc chemistry materials-science

librl's People

Contributors

Stargazers

Watchers

Forkers

sulstice nicholasmais lucas-xmc

librl's Issues

Documentation refactoring suggestions

Again for the JOSS review.
I like that you use sphinx and readthedocs for the documentation and have a lot of comments and docstrings.

One idea for improvement of the documentation would be to first give some more introduction and walk the reader through one example (show input, output, reference to theory) and then have some automatically generated API documentation (sphinx.ext.napoleon) better separated from the installation and examples.

Paths do not work on all OS

Hi, as part of the JOSS review I'll open some small 'bug' issues and later one with more comments.

The first one is that the way you join paths, e.g, in the examples.py does not work on all operating systems. It would be better if you use an OS agnostic way (os.path.join).

Contribution guide is missing

Another point for the review for JOSS: You should probably add a contribution guide. See the Github documentation.

JOSS review general comments

Hi, just some general comments on your package, to summarize my review. I'm not an expert in RAMs but your package seems to be useful for your community and the software paper does its job in summarizing this need and you also highlight your previous works in it. It also seems to work, besides the minor path issue.

I think the major points that would make it more user-friendly is to add some verbosity to an examples (see issue #7 or #6). You could even think about adding an installable command line interface as entry point, e.g. using click to make running the analysis a one-liner (at least for the next release). For developer friendliness, you can have a look at some of output of pylint or prospector beyond adding the contribution guidelines.

There are some issues that I couldn't check (yet) on the review checklist like automatic tests, contribution guidelines and the licence. But they seem to be relatively easy to fix.
You might also consider adding your package to pypi.

PEP8 / linting and autoformatting

Hi, a minor point I noticed looking at the code (for the JOSS review) is that it does follow PEP8 in all places.

Even though it is not an explicit criterion it makes working with the code easier if one sticks to PEP8.

This can be done relatively easily with some auto formatting upon each commit using e.g. the pre-commit library which you can then use to run linters like prospector and code formatters like yapf or black. You could also use this to automatically run tests (locally) upon each commit.

In particular, functions should be lowercase according to PEP8 and also type checks might be better written in duck type flavor in try/except cases or if isinstance rather than with type (see e.g. stackoverflow)

Below, I'm just copy-pasting the prospector output so you can give a look if you think that there are points that are reasonable to fix. Most of them are really minor.

Messages
========

libRL/__init__.py
  Line: 44
    pylint: wrong-import-position / Import "import pyximport" should be placed at the top of the module
  Line: 46
    pep8: E275 / missing whitespace after keyword (col 18)
    pylint: import-self / Module import itself
    pylint: wrong-import-position / Import "from libRL import refactoring, quick_graphs, cpfuncs" should be placed at the top of the module
  Line: 52
    pylint: redefined-builtin / Redefining built-in 'abs'
    pylint: wrong-import-position / Import "from numpy import arange, zeros, abs, array, float64, errstate, pi, sqrt" should be placed at the top of the module
  Line: 57
    pylint: wrong-import-position / Import "from pandas import DataFrame" should be placed at the top of the module
  Line: 58
    pylint: wrong-import-position / Import "from pathos.multiprocessing import ProcessPool as Pool" should be placed at the top of the module
  Line: 236
    pylint: literal-comparison / Comparison to literal (col 11)
  Line: 291
    pylint: too-many-locals / Too many local variables (19/15)
  Line: 387
    pylint: unidiomatic-typecheck / Using type() instead of isinstance() for a typecheck. (col 7)
  Line: 486
    pylint: too-many-locals / Too many local variables (17/15)
  Line: 632
    pep8: E306 / expected 1 blank line before a nested definition, found 0 (col 5)

libRL/quick_graphs.py
  Line: 29
    pylint: unused-import / Unused mpl_toolkits.mplot3d.axis3d imported as axis3d
  Line: 64
    pylint: protected-access / Access to a protected member _axinfo of a client class (col 8)
  Line: 120
    pep8: E722 / do not use bare 'except' (col 5)
  Line: 127
    pep8: E722 / do not use bare 'except' (col 5)
  Line: 173
    pep8: E722 / do not use bare 'except' (col 5)
  Line: 180
    pep8: E722 / do not use bare 'except' (col 5)
  Line: 185
    pylint: trailing-newlines / Trailing newlines

libRL/refactoring.py
  Line: 38
    pylint: redefined-builtin / Redefining built-in 'abs'
  Line: 99
    pylint: bare-except / No exception type(s) specified (col 20)
  Line: 164
    pylint: literal-comparison / Comparison to literal (col 30)
  Line: 224
    pylint: literal-comparison / Comparison to literal (col 9)
  Line: 238
    pylint: literal-comparison / Comparison to literal (col 9)
  Line: 276
    pylint: no-else-raise / Unnecessary "elif" after "raise" (col 4)
  Line: 281
    pylint: unidiomatic-typecheck / Using type() instead of isinstance() for a typecheck. (col 9)
  Line: 289
    pep8: E722 / do not use bare 'except' (col 9)
  Line: 317
    pylint: unidiomatic-typecheck / Using type() instead of isinstance() for a typecheck. (col 7)
  Line: 323
    pep8: E722 / do not use bare 'except' (col 9)
  Line: 337
    pylint: no-else-raise / Unnecessary "else" after "raise" (col 4)



Check Information
=================
         Started: 2019-11-07 09:11:28.638680
        Finished: 2019-11-07 09:11:39.781930
      Time Taken: 11.14 seconds
       Formatter: grouped
        Profiles: default, strictness_medium, strictness_high, strictness_veryhigh, no_doc_warnings, no_test_warnings, no_member_warnings
      Strictness: medium
  Libraries Used:
       Tools Run: dodgy, mccabe, pep8, profile-validator, pyflakes, pylint
  Messages Found: 31

and the output from just running pylint

************* Module libRL
__init__.py:211:0: C0330: Wrong continued indentation (add 41 spaces).
            (e1f(f) - cmath.sqrt(-1) * e2f(f)))) * (cmath.tanh(j *
            ^                                        | (bad-continuation)
__init__.py:212:0: C0330: Wrong continued indentation (add 51 spaces).
            (2 * cmath.pi * (f * 10**9) * (d * 0.001) / 299792458) *
            ^                                                  | (bad-continuation)
__init__.py:213:0: C0330: Wrong continued indentation (add 51 spaces).
            cmath.sqrt((mu1f(f) - j * mu2f(f)) * (e1f(f) - j *
            ^                                                  | (bad-continuation)
__init__.py:214:0: C0330: Wrong continued indentation (add 38 spaces).
            e2f(f)))))) - 1) / ((1 * (cmath.sqrt((mu1f(f) - j * mu2f(f)) /
            ^                                     | (bad-continuation)
__init__.py:215:0: C0330: Wrong continued indentation (add 37 spaces).
            (e1f(f) - j * e2f(f)))) * (cmath.tanh(j * (2 *
            ^                                    | (bad-continuation)
__init__.py:216:0: C0330: Wrong continued indentation (add 43 spaces).
            cmath.pi * (f * 10**9) * (d * 0.001) / 299792458) * cmath.sqrt(
            ^                                          | (bad-continuation)
__init__.py:217:0: C0330: Wrong hanging indentation (add 4 spaces).
            (mu1f(f) - j * mu2f(f)) * (e1f(f) - j *
            ^   | (bad-continuation)
__init__.py:218:0: C0330: Wrong continued indentation (add 27 spaces).
            e2f(f)))))) + 1)))))
            ^                          | (bad-continuation)
__init__.py:249:0: C0330: Wrong hanging indentation before block.
            ) is True:
    |   |   ^ (bad-continuation)
__init__.py:271:0: C0330: Wrong hanging indentation (remove 24 spaces).
                                        i * gridInt:(i + 1) * gridInt, 0:3
                |                       ^ (bad-continuation)
__init__.py:272:0: C0330: Wrong hanging indentation.
                                        ]
            |   |                       ^ (bad-continuation)
__init__.py:635:0: C0330: Wrong continued indentation (add 5 spaces).
            (e1f(f) - cmath.sqrt(-1) * e2f(f))).real)) * (
            ^    | (bad-continuation)
__init__.py:656:0: C0330: Wrong hanging indentation before block.
            ) is True:
    |   |   ^ (bad-continuation)
__init__.py:659:19: C0326: No space allowed around keyword argument assignment
            d_vals = d_set,
                   ^ (bad-whitespace)
__init__.py:660:19: C0326: No space allowed around keyword argument assignment
            m_vals = m_set,
                   ^ (bad-whitespace)
__init__.py:52:0: W0622: Redefining built-in 'abs' (redefined-builtin)
__init__.py:1:0: C0103: Module name "libRL" doesn't conform to snake_case naming style (invalid-name)
__init__.py:42:0: C0103: Constant name "here" doesn't conform to UPPER_CASE naming style (invalid-name)
__init__.py:44:0: C0413: Import "import pyximport" should be placed at the top of the module (wrong-import-position)
__init__.py:44:18: C0321: More than one statement on a single line (multiple-statements)
__init__.py:46:0: C0413: Import "from libRL import refactoring, quick_graphs, cpfuncs" should be placed at the top of the module (wrong-import-position)
__init__.py:46:0: W0406: Module import itself (import-self)
__init__.py:52:0: C0413: Import "from numpy import arange, zeros, abs, array, float64, errstate, pi, sqrt" should be placed at the top of the module (wrong-import-position)
__init__.py:57:0: C0413: Import "from pandas import DataFrame" should be placed at the top of the module (wrong-import-position)
__init__.py:58:0: C0413: Import "from pathos.multiprocessing import ProcessPool as Pool" should be placed at the top of the module (wrong-import-position)
__init__.py:61:0: C0103: Function name "RL" doesn't conform to snake_case naming style (invalid-name)
__init__.py:61:0: C0103: Argument name "Mcalc" doesn't conform to snake_case naming style (invalid-name)
__init__.py:205:4: C0103: Function name "G" doesn't conform to snake_case naming style (invalid-name)
__init__.py:206:8: C0103: Variable name "f" doesn't conform to snake_case naming style (invalid-name)
__init__.py:207:8: C0103: Variable name "d" doesn't conform to snake_case naming style (invalid-name)
__init__.py:210:8: C0103: Variable name "y" doesn't conform to snake_case naming style (invalid-name)
__init__.py:236:11: R0123: Comparison to literal (literal-comparison)
__init__.py:259:8: C0103: Variable name "gridInt" doesn't conform to snake_case naming style (invalid-name)
__init__.py:264:8: C0103: Variable name "MCres" doesn't conform to snake_case naming style (invalid-name)
__init__.py:291:0: C0103: Function name "CARL" doesn't conform to snake_case naming style (invalid-name)
__init__.py:291:0: C0103: Argument name "Mcalc" doesn't conform to snake_case naming style (invalid-name)
__init__.py:291:0: R0914: Too many local variables (19/15) (too-many-locals)
__init__.py:387:7: C0123: Using type() instead of isinstance() for a typecheck. (unidiomatic-typecheck)
__init__.py:394:4: C0103: Variable name "c" doesn't conform to snake_case naming style (invalid-name)
__init__.py:395:4: C0103: Variable name "GHz" doesn't conform to snake_case naming style (invalid-name)
__init__.py:396:4: C0103: Variable name "Z0" doesn't conform to snake_case naming style (invalid-name)
__init__.py:397:4: C0103: Variable name "e0" doesn't conform to snake_case naming style (invalid-name)
__init__.py:467:4: C0103: Variable name "Matrix" doesn't conform to snake_case naming style (invalid-name)
__init__.py:486:0: C0103: Function name "BARF" doesn't conform to snake_case naming style (invalid-name)
__init__.py:486:0: C0103: Argument name "Mcalc" doesn't conform to snake_case naming style (invalid-name)
__init__.py:486:0: R0914: Too many local variables (17/15) (too-many-locals)
__init__.py:621:4: C0103: Variable name "PnPGrid" doesn't conform to snake_case naming style (invalid-name)
__init__.py:632:4: C0103: Argument name "f" doesn't conform to snake_case naming style (invalid-name)
__init__.py:632:4: C0103: Argument name "m" doesn't conform to snake_case naming style (invalid-name)
__init__.py:633:8: C0103: Variable name "y" doesn't conform to snake_case naming style (invalid-name)
__init__.py:640:4: C0103: Variable name "mGrid" doesn't conform to snake_case naming style (invalid-name)
__init__.py:643:11: C0103: Variable name "m" doesn't conform to snake_case naming style (invalid-name)

Tests could be more fine-grained and automatized

Hi, again for the JOSS review.

I saw that you have a test case for the code, which is great! If you want to improve on that you can write unit tests that test each (relevant) function (this makes it easier for new contributors to see where exactly they break your code) and run those tests automatically, e.g. with pytest on different platforms using e.g. travis or azure pipelines.

LICENSE file not plain text and not recognized by pip

Hi, again as part of the JOSS review.

Your license is not a plaintext file (LICENSE), but a markdown. this is likely the reason also GitHub does not show it in the upper right corner.

Also, pip does not show the license, in my case the output of pip show is

Name: libRL
Version: 1.0.3
Summary: Library of functions used for characterizing Microwave Absorption
Home-page: https://github.com/1mikegrn/libRL
Author: Michael Green
Author-email: [email protected]
License: UNKNOWN
Location: /Users/kevinmaikjablonka/anaconda3/envs/joss_librl/lib/python3.7/site-packages
Requires: numpy, pandas, pathos, xlrd, cython, scipy, matplotlib
Required-by:

Example suggestions

Here's one more issue for the JOSS review.

You have an example and it seems to work (besides the path issue). To make it even better you can consider having some more comments on it (e.g., a section in the docs that also shows the output plots one should find in the end) or even a Jupyter notebook with plots and markdown fields that describe what happens (and which can run on binder) to show people what the code can do without them having to install the package.

Recommend Projects

React

A declarative, efficient, and flexible JavaScript library for building user interfaces.
Vue.js

🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
Typescript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
TensorFlow

An Open Source Machine Learning Framework for Everyone
Django

The Web framework for perfectionists with deadlines.
Laravel

A PHP framework for web artisans
D3

Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

javascript

JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
web

Some thing interesting about web. New door for the world.
server

A server is a program made to process requests and deliver data to clients.
Machine learning

Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Visualization

Some thing interesting about visualization, use data art
Game

Some thing interesting about game, make everyone happy.

Recommend Org

Facebook

We are working to build community through open source technology. NB: members must have two-factor auth.
Microsoft

Open source projects and samples from Microsoft.
Google

Google ❤️ Open Source for everyone.
Alibaba

Alibaba Open Source for everyone
D3

Data-Driven Documents codes.
Tencent

China tencent open source team.