Create a class to represent physical types by namurphy · Pull Request #11204 · astropy/astropy

namurphy · 2021-01-04T03:29:59Z

This pull request creates a class, units.physical.PhysicalType, that represents the physical type(s) associated with a set of units. The overall purpose is to allow there to be multiple physical types associated with a particular set of units. This functionality could allow for pressure having equivalent units to energy density. Instances of this class will become the physical_type attribute of Unit instances. The dunder methods were written to mostly preserve backward compatibility, and also allow for some dimensional analysis.

The motivation and general API for this is described in #11202. Closes #11202. The idea for this came about while working on #11200 and PlasmaPy/PlasmaPy#859.

I did notice and fix one minor bug: the physical type for moles per cubic meter was corrected to "molar concentration". It was previously "molar volume", which was its inverse.

Things I still need to do:

Expand tests (situations with unknown or multiple physical types)
Expand the docstring of PhysicalType (parameters, examples including of dunder methods, etc.)
Write a changelog entry ~~in iambic pentameter~~
Define multiple physical types ~~(perhaps mostly in a follow-up pull request that will supersede Add new physical types and parametrize physical type tests #11200)~~

In #11202 the idea came up to have this work with equivalencies. I'm not quite sure if or how we should deal with that since it could get kind of messy and/or ambiguous. Oh well, that's for another day.

Thermal conductivity, heat capacity, specific heat capacity, energy flux, molar volume (had been previously misdefined), electrical resistivity, electrical conductivity, magnetic dipole moment, volumetric rate, jerk, snap, crackle, pop, temperature gradient, specific energy, reaction rate, moment of inertia, catalytic activity, molar heat capacity, molality, absement, volumetric flow rate, frequency drift, compressibility. These changes were already proposed in astropy#11200, which was supplanted by the current pull request (astropy#11204). This commit was brought to you by needing an excuse to procrastinate for an hour on Wikipedia.

pep8speaks · 2021-01-06T04:27:24Z

Hello @namurphy 👋! It looks like you've made some changes in your pull request, so I've checked the code again for style.

There are no PEP8 style issues with this pull request - thanks! 🎉

Comment last updated at 2021-04-16 18:54:54 UTC

mhvk · 2021-01-06T16:11:12Z

@namurphy - this sounds interesting. But before I dive in, is it ready for review, or are you still changing things around?

namurphy · 2021-01-06T16:57:32Z

@mhvk — thanks! It's ready for review. At this point the only likely changes are expanding the physical types...in particular for cases where the same set of units can correspond to multiple physical types.

Some things to look out for in code review:

Physical types for temperature units: Though K, °C, °F, K, and °Ra are not directly convertible, they all correspond to the same physical type.
Definitions of new physical types: The last ~20 physical types defined in _units_and_physical_types in physical.py are new. I put in absement and absity in there which are integrals of the position vector, but for the moment I decided against including even higher order integrals for which names have been proposed but aren't in wide use. I'm guessing the higher order integrals will probably only be used by integral kinematics hobbyists.
How equality is handled: I tried to keep the existing API for physical types as much as possible, while also expanding it.
Should the objects in physical.py be made private?: This module states that it is not intended for use by the user, in which case it might make sense to deprecate them. At the same time, the documentation for PhysicalType especially is probably worth having public.

All in all, this was a pretty fun pull request to write to give me an excuse to procrastinate doing other things. Thanks again!

mhvk

@namurphy - This looks very nice indeed! Quite a number of comments in-line. In terms of implementation, the main ones are to let str(physical_type) and set(physical_type) be the simple ways for the current as_set and as_string properties (which I do not like so much).

More general:

Would it be possible not to use typing yet? I've no strong opinion myself but feel it is something the project should decide on how to do it; for instance, it may be that we decide to go the numpy route of keeping this out of the code proper and work with stubs instead. (Also, there is a more specific problem that the units are not in fact instances of u.Unit...)
I prefer EAFP over LBYL myself, so would not worry so much about all the isinstance checks. This especially since it is code used internally. But it is just a preference!

On your specific points:

I think your work-around for temperature is quite good. For now, it is nicely an implementation detail - perhaps we will eventually think of something better!
I like the units you added, though am a bit less sure about 'count', 'pixel', etc., as "number of ..." does not really feel like a physical type.
I like how you handle equality!
I don't think the objects should be made private. The two functions were already public and I think it is fine to add the new class. It also means we can refer to PhysicalType elsewhere if needed. Regarding that, do you think we should add some of your examples in its docstring to the actual docs? Looking at the docs, we basically have next to nothing right now... Overall, my sense would be to do this as a follow-up PR, leaving this one as a supernice one about implementation.

mhvk · 2021-01-06T21:44:48Z

+    (u.m * u.s ** 8, "unknown"),
+    (u.m / u.m, "dimensionless"),
+    (hbar.unit, "angular momentum"),
+    (hbar.unit, "angular momentum"),


duplication

🙀 Thanks!

mhvk · 2021-01-06T21:50:03Z


+def test_physical_type_as_set(length, pressure):
+    """Test the `as_set` attribute of `physical.PhysicalType`."""
+    assert length.as_set == {"length"}


Hmm, maybe just allow set(length)? (just looking at tests for now...)

Good idea! I suppose the best way to do this is to set __iter__ and __next__ and make it iterable, unless there's another way I'm unaware of.

Ha, yes, as you'll see below! (I tried, you need __iter__ only.)

mhvk · 2021-01-06T21:54:38Z

-def test_speed():
-    assert (u.km / u.h).physical_type == 'speed'

+@pytest.fixture


Not a big deal, but to me these fixtures obscure the tests a bit; not even sure all are used!

mhvk · 2021-01-06T21:59:21Z

+`physical_type` property.
 """

+__all__ = ["def_physical_type", "get_physical_type", "PhysicalType"]


Could you move __all__ back down? It seems the standard place within astropy.

Oops, force of habit! In PlasmaPy we ended up having the __all__ before as a PEP 8 thing. I'll move it back.

mhvk · 2021-01-07T00:15:29Z

+    _physical_unit_mapping[physical_type_id] = physical_type

-def get_physical_type(unit):
+    for ptype in physical_type.as_set:


With __iter__ defined as above, you can just iterate directly in physical_type.

mhvk · 2021-01-07T00:16:06Z

-def get_physical_type(unit):
+    for ptype in physical_type.as_set:
+        if ptype in _unit_physical_mapping.keys():
+            raise ValueError(f"{ptype} has already been defined")


Try to cover this?

mhvk · 2021-01-07T00:18:54Z

+
+    physical_type_id = unit._get_physical_type_id()
+
+    known_unit = physical_type_id in _physical_unit_mapping


I'd either do

ptype = _physical_unit_mapping.get(physical_type_id) if ptype is not None: ...

or put the expression directly in the if statement.

mhvk · 2021-01-07T00:39:16Z

+            other = self._standardize_physical_type_names(other)
+
+        if isinstance(other, set):
+            return other.issubset(self.as_set)


Isn't other.intersection(self.as_set) good enough?

Yeah...that makes a bit more sense, come to think of it. Like, if someone is looking for physical types of "mass" and "force" they might want to check if the physical type is either of those things. I should also document this.

Ha, I did not actually think of that, somehow! Just that they might have a set of names for the same unit too. I now wonder if we're not being too generous here. Perhaps we should not allow comparison with a set of strings at all?

mhvk · 2021-01-07T00:39:51Z

+
+        if isinstance(other, set):
+            return other.issubset(self.as_set)
+        elif isinstance(other, PhysicalType):


One of those totally trivial things, but I'd put this first as it is the comparison that should be easiest to deal with.

mhvk · 2021-01-07T00:56:50Z

Ah, and of course this needs a changelog entry in CHANGES.rst, under 4.3, units

mhvk

Few more comments, though when I started going I realized it was not quite ready for the next round...

mhvk · 2021-01-09T15:57:19Z

 ^^^^^^^^^^^^^

+- Create the ``astropy.units.physical.PhysicalType`` class to represent the
+  physical types of units and all there to be more than one physical type


all -> allow

mhvk · 2021-01-09T15:59:20Z


+- The ``physical_type`` attribute of units now returns an instance of
+  ``astropy.units.physical.PhysicalType`` instead of a string.  This
+  class was written to largely maintain the existing API.  [#11204]


Maybe add "PhysicalType instances can be compared to strings, so no code changes should be necessary."

mhvk · 2021-01-09T16:01:15Z


+    Many of these tests are used to test backwards compatibility.
+    """
+    if unit.physical_type != physical_type:


Why not just assert unit.physical_type == physical_type? Pytest will then list essentially the failure message added here, no?

mhvk · 2021-01-11T19:30:32Z

@namurphy - the test failure on 32-bit looks related. I have not looked in detail, but maybe something got changed and not put back or so.

Otherwise, let me know when ready for the next round of review!

namurphy · 2021-01-15T03:11:58Z

@mhvk — thanks! I addressed most or all of your code review suggestions, so it's ready for another round of review.

One change is that I decided to let def_physical_type expand the names of physical types for a PhysicalType instance that has already been defined for a unit. I did this thinking about backward compatibility, since there are a few uses of def_physical_type that I found on GitHub that define physical types that are being added in this PR, though with different names. This change will make it less likely to break code in those packages.

I also made PhysicalType instances hashable, since I figured I might end up using them as dict keys here and there.

mhvk

@namurphy - I like the idea to allow adding new physical types to existing units, as well as making them hashable.

Still quite a lot of comments. For the operators, mostly to ensure you return NotImplemented instead of raising (should have caught that earlier); for tests, nitpicks, but perhaps good to organize those a bit more.

The other things are probably more philosophical: I'm very much a "Easier to Ask Forgiveness than Permission" coder, and avoid "Look before you leap" isinstance checks - I like duck-typing, making the most minimal assumptions possible! And in non-user facing code it seems just not very necessary.

mhvk · 2021-01-16T01:58:28Z

-
+        >>> u.m.physical_type
+        'length'
+        >>> u.m.physical_type ** 3


I'd maybe remove this example here, leaving it for the (nice) more detailed examples later.

mhvk · 2021-01-16T02:02:38Z

+
+
+def test_expanding_names_for_physical_type():
+    """


The docstring is wrong here. Also, you need to be sure to remove the definition from the physical types dict, i.e., some try/finally construct or setup/teardown (it may be an idea to move some functions in a test class with setup_class and teardown_class methods).

mhvk · 2021-01-16T02:04:10Z

+
+
+def test_multiple_same_physical_type_names():
+    """Test that def_physical_type does not define a """


Incomplete docstring.

Oops! I have a bad habit of starting sentences and then forgetting to

mhvk · 2021-01-16T02:07:00Z

+    (si.A / si.m ** 2, "electrical current density"),
+    (si.V / si.m, "electrical field strength"),
+    (
+        si.C / si.m ** 2,


Why so weirdly offset? I'd just indent the set by 4 spaces, and keep the closing parenthesis behind it (or break after the set items)

Oh...I got in the habit of using black for formatting and have gotten accustomed to that style. I'll change this to match the Astropy style.

Yes, I noticed it is taking python programmers by storm... I'm still a bit on the fence, feeling the urge to minimize line changes is coming at the cost of readability. Though I can see the case for consistency. Anyway, some of the others are quite allergic to it!

mhvk · 2021-01-16T02:22:45Z

+    elif isinstance(obj, PhysicalType):
+        return obj._unit
+    else:
+        raise TypeError("Expecting a unit or a physical type")


Actually, since this is used in methods, you should not raise but return NotImplemented (after all, it is possible other knows how to deal with a PhysicalType even though we do not know how to deal with other). Probably another reason to do it on the class...

mhvk · 2021-01-16T02:23:38Z

+            other = _standardize_physical_type_names(other)
+            return other.issubset(self._physical_type)
+        else:
+            return False


Strictly, you should return NotImplemented here (and adjust __ne__ to take care of that as needed)

mhvk · 2021-01-16T02:29:01Z

+        else:
+            return "{" + str(sorted(self._physical_type))[1:-1] + "}"
+
+    def __mul__(self, other):


To be able to return NotImplemented as needed, I would write a new method, like

def _dimensional_analysis(self, other, op): # Maybe easiest to just include function in here. other_unit = _identify_unit_from_unit_or_physical_type(other) if other_unit is NotImplemented: return NotImplemented other_unit = _replace_temperatures_with_kelvin(other_unit) new_unit = getattr(self._unit, op)(other_unit) # Don't think we can get NotImplemented here any more return new_unit.physical_type

and then

def __mul__(self, other): return self._dimensional_analysis(other, '__mul__') def __rmul__.... def __truediv__.... def __rtruediv__....

Oh! I like this!

mhvk · 2021-01-16T02:35:43Z

-    (cgs.abcoulomb, 'electrical charge (EMU)')
-]:
-    def_physical_type(unit, name)
+    if not isinstance(unit, core.UnitBase):


I'm still not sold on these checks for non-user code - just let the replacement fail?

maxnoe · 2021-01-19T10:08:25Z

+        'length'
+
+        >>> u.Pa.physical_type
+        {'energy density', 'pressure', 'stress'}


I strongly disagree. A Pascal is not a unit of energy density (J / m³). It just decomposes to the same SI base units.

Hmm, of course you are right and I guess the more appropriate physical example would be (u.kg/(u.m*u.s**2)).physical_type. Somehow, I've always "decomposed in my head...".

In terms of implementation, this is tricky, though - I'm not sure how we can distinguish, since everything right now goes through _physical_type_id. Possibly, individual given units could be associated with a given physical type.

maxnoe · 2021-01-19T13:30:30Z

I have to issues with this:

By going through decomposition, special units loose their semantic meaning, which should be avoided
physical_type is now sometimes a string, sometimes a set. This should be consistent (give a set of length one if there is only one possibility). This avoids the need for checking what you got and won't break code if we realize we forgot a meaning.

mhvk · 2021-01-19T13:51:06Z

@maxnoe - just to be clear, there is the reverse problem that this is trying to solve:

u.Unit("kg / (m s2)").physical_type
'pressure'

Internally, physical_type now already always is a set - is your concrete suggestion to let str and repr reflect that? The reason that for length 1 they look like strings is for backwards compatibility. Similarly, for that reason __eq__ is really defined as whether there is an intersection.

maxnoe · 2021-01-19T14:16:16Z

@mhvk ok, good, then I was only confusing the repr with the actual thing in the examples.

Which brings about another point then: the common python way is that repr(thing) identifies really what an object is, and only str should simplify this for nice printing.

So repr should probably be something like PhysicalType({length, }), not 'length' and str can then simplify for things that only have one type.

mhvk · 2021-01-19T14:34:37Z

True, main problem of course is that changing __repr__ that way will break doctests, etc. But perhaps worth it for the enhanced clarity...

namurphy · 2021-01-19T20:53:46Z

the common python way is that repr(thing) identifies really what an object is, and only str should simplify this for nice printing.

My intention was to maintain backwards compatibility with the previous API of having physical_type be a string, though I personally would prefer it to have repr be something more descriptive like you suggested. As @mhvk mentioned, this could end up breaking doctests, in particular in downstream packages. This change would probably be straightforward enough for other packages to fix, so I'd be okay with a minor break in backwards compatibility to match Python conventions. If we have agreement, I'll go ahead and change __repr__ when next I have a chance to work on this.

mhvk · 2021-01-19T21:38:30Z

On balance, I think a PhysicalType(...) repr is better. I think most bigger packages that have documentation would test against astropy-dev, so we'd hopefully find out about problems before an actual release.

namurphy · 2021-01-20T23:49:45Z

I changed PhysicalType.__repr__ to return strings like PhysicalType('length') for things with one physical type, and strings like PhysicalType({'absement', 'sustained displacement'}) when there is more than one physical type. I'd be okay with having both cases come back with set notation, or both without the set notation. I put a note in the changelog that stringifying physical types is not backwards compatible. Updating the doctests was pretty straightforward. This still could break doctests in downstream packages, but the fix for the broken doctests is pretty straightforward. I like the improved clarity.

I changed the docstrings to mention that PhysicalType instances represent the physical types of "dimensionally compatible" units, which provides context for units like pascals.

@mhvk — when you have a chance, could you partake in another round of code review? Thanks in advance!

namurphy · 2021-01-21T01:52:21Z

Update: I fell down a rabbit hole and am fixing a few more things, so it'd be best to review this January 21 or later.

namurphy · 2021-01-21T14:45:25Z

+    def __array_ufunc__(self):
+        # This prevents operations like Unit(...) * PhysicalType(...)
+        # from returning a Quantity with a PhysicalType instance as the
+        # Quantity's value.
+        pass


My motivation for adding this method was to try ways to prevent things like this from happening:

>>> u.m * u.m.physical_type <Quantity PhysicalType('length') m>

I wanted Unit.__mul__ to return NotImplemented here so that this operation would fall back on PhysicalType.__rmul__, ideally without making any changes to Unit or Quantity. I speculated that having PhysicalType.__array_ufunc__ raise a TypeError would prevent this from happening, but it appears to only be necessary for PhysicalType.__array_ufunc__ to exist without actually doing anything. It's a bit confusing why this works, so I'm curious if there is a more understandable way to ensure this behavior.

I'll think about this - I think the underlying problem is np.array(physical_type) creating an object array. __array__ = None may be better.

Thanks! I'll go with that for the time being.

mhvk

OK, went for another round. It is looking good. The only more major comment is that I'm not sure why in the dimensional analysis numbers are allowed.

Also, maybe time to squash the commits?

mhvk · 2021-01-31T01:26:40Z

+    assert obscure_unit.physical_type == "unknown"
+
+
+def _undef_physical_type(unit):


I think the way to do this is with teardown_function - that way it is guaranteed to run also if the test fails.

(I think all this is easier with a test class as suggested in https://github.com/astropy/astropy/pull/11204/files#r558759530 below)

namurphy · 2021-01-31T17:33:13Z

@mhvk — Thank you! Your comments have been really helpful in improving this pull request on both the small scale code changes as well as the larger overall structure. I'll hopefully have a chance to implement the suggestions by early this week.

Also, maybe time to squash the commits?

That does seem like an increasingly good idea. We could either squash and merge when the PR is ready, or I could do a rebase and squash commits individually.

nstarman · 2021-04-20T18:12:38Z

@pllim. Alternatively, for backward compatibility we could mod __contains__ in PhysicalType..

pllim · 2021-04-20T18:18:35Z

If I am the only one affected, there is no need, as I already patched it downstream. Not sure about other users. @karllark , are you affected by this change?

nstarman · 2021-04-20T18:19:17Z

something like:

def __contains__(self, other):
    contains = super().__contains__(other)
    if not contains:
        warnings.warn("deprecated", AstropyDeprecationWarning)  # ?
        contains |= any([other in x for x in self])
    return contains

karllark · 2021-04-20T18:22:41Z

If I am the only one affected, there is no need, as I already patched it downstream. Not sure about other users. @karllark , are you affected by this change?

I don't think this affects me. I haven't used this capability to my knowledge.

rosteen · 2021-04-20T20:18:47Z

For what it's worth, this also breaks Jdaviz in a few places where we were using unit.physical_type.title to populate axis labels. We can fix it, but I wouldn't be surprised if there are other places throughout the ecosystem explicitly treating physical_type as a string.

mhvk · 2021-04-20T21:57:15Z

Hmm, two reports when not even released makes me think that perhaps we should add something like

def __getattr__(self, attr):
    self_str_attr = getattr(str(self), attr, None)
    if self_str_attr is not None:
        warnings(... DeprecationWarning)
        return self_str_attr
    return super().__getattribute__()  # Raise AttributeError

With the deprecation error mentioning that if one relied on string attributes, one could just do str(physical_type).attr (which would be backwards compatible).

Or perhaps this is still rare enough that we can just put it in the docs (and the what's new entry)?

astrofrog · 2021-04-21T14:44:11Z

Yes I agree that we should transparently make string methods work on the new physical type class, with a deprecation warning. I'm pretty sure I've also relied on physical_type being a string in some of my code.

namurphy · 2021-04-21T16:54:49Z

I just created #11617 on addressing this. Thank you for raising these issues, and my apologies for breaking things!

Because the physical_type attribute of a unit was a str before astropy#11204, there may be code that depends on those str methods for the physical_type attribute. This approach was adapted from code suggested in astropy#11204 after it was merged. Co-authored-by: Marten van Kerkwijk <mhvk@astro.utoronto.ca>

Parametrize and expand unit physical type tests Now pretty much all of the existing unit/physical type pairs are being tested in advance of the impending refactoring. These tests will check backward compatibility. Define __eq__, __ne__, and __contains__ in PhysicalType __eq__ and __ne__ are being written so as to preserve the current API to the extent that is possible. For example, if we do `PhysicalType(u.m, "length") == "length"`, it will return True. Similarly, we are now additionally able to do things like `"length" in PhysicalType(u.m, "length")` and have it return True also. Define __repr__ and __str__ for PhysicalType This too is to preserve backward compatibility. If there is only one physical type, then these methods will return the string of the physical type. If there is more than one physical type, then these methods will stringify the set containing the multiple physical types. Define __mul__, __truediv__, & __pow__ for PhysicalType Multiplication, division, and exponentiation can be done for physical types analogously to units, like volume = length * area or density of wombats times velocity of wombats equals flux of wombats. This would enable a form of dimensional analysis. Add new physical types Thermal conductivity, heat capacity, specific heat capacity, energy flux, molar volume (had been previously misdefined), electrical resistivity, electrical conductivity, magnetic dipole moment, volumetric rate, jerk, snap, crackle, pop, temperature gradient, specific energy, reaction rate, moment of inertia, catalytic activity, molar heat capacity, molality, absement, volumetric flow rate, frequency drift, compressibility. These changes were already proposed in astropy#11200, which was supplanted by the current pull request (astropy#11204). Update class that represents physical types - Use kelvin to represent all temperatures internally. Celsius, Fahrenheit, and Rankine temperatures are not directly convertible to kelvin, but these all represent the same physical type. - Updated docstrings of PhysicalType. Add a warning banner that PhysicalType is not intended to be instantiated by the user. - Allow string representations of physical types to have underscores instead of spaces. If someone is working with an object that has an attribute that is named after a physical type, then the words in that physical type will probably be separated by underscores. Using underscores will simplify the code for this a bit. (I'm planning on using this feature in PlasmaPy). - Allow string representations of physical types to contain multiple physical types that are separated by a backslash or a comma. This is to maintain backward compatibility (for example with "momentum/impulse" which was a physical type before this pull request). and to contain multiple physical types that are separated by a backslash or a comma. Expand and refine physical type tests - Include more tests of temperature - Test that string representations of different physical types work (e.g., that underscores are treated as spaces and that strings can have multiple physical types separated by commas or backslashes). Refinements following code review - Change Jansky physical type back - Clean up docstrings - Remove type annotations - Make PhysicalType iterable over different physical type names (sorted) - Test iteration over PhysicalType - Remove `as_string` attribute in favor of using str(physical_type) - Remove `as_set` attribute in favor of using set(physical_type) - When necessary, use `_as_set` attribute internally - Remove duplication in parametrized test - Remove fixtures from tests - Test that a physical type name cannot be used for multiple units, except for unknown Clean up PhysicalType class & tests This includes changing an exception being raised with a warning being issued when a unit's physical type is being redefined. Update physical types and tests - Expanded definitions of physical types - Edited PhysicalType docstring to include discussion of comparisons and how physical type names are processed. Also revised examples. - No longer allow physical type names in a single string to be separated by a comma, while stating that the slash separator is kept for backwards compatibility. - Removed `_get_physical_type_id` method. Now using a private attribute instead. - Remove feature of allowing PhysicalType instances to be compared with sets that may contain strings representing physical types. Clean up PhysicalType class - Simplify `__iter__` by using `yield` - Rename some private attributes Raise ValueError when trying to redefine unit Also added a note to docstrings of `get_physical_type` and `def_physical_type` that these are not intended for use by users.

astrofrog · 2021-05-02T07:48:56Z

Another downstream package whose tests are broken with the current astropy latest: radio-astro-tools/spectral-cube#709 - is there any way to fix this? Could we perhaps make sure the hash is the same as the string hash?

Ping @mhvk @namurphy

astropy/astropy#11204 removed `imperial` from being imported into `astropy.units` namespace

pllim · 2021-07-27T12:44:31Z

 from . import si
 from . import astrophys
 from . import cgs
-from . import imperial


Looking at #11975 , it is unclear to me now why this line was removed...

Oh my. This was a huge mistake on my part. I apologize for that. I have no idea how or why I let that slip in there....

Probably a tool warned "unused import"?

github-actions Bot added the units label Jan 4, 2021

pllim added the Enhancement label Jan 4, 2021

namurphy marked this pull request as ready for review January 5, 2021 02:44

namurphy mentioned this pull request Jan 6, 2021

Add new physical types and parametrize physical type tests #11200

Closed

mhvk reviewed Jan 7, 2021

View reviewed changes

mhvk added this to the v4.3 milestone Jan 7, 2021

namurphy mentioned this pull request Jan 8, 2021

ParticleTracker refactor: Bring in particle_integrators.py PlasmaPy/PlasmaPy#953

Merged

mhvk reviewed Jan 9, 2021

View reviewed changes

mhvk reviewed Jan 16, 2021

View reviewed changes

maxnoe reviewed Jan 19, 2021

View reviewed changes

namurphy commented Jan 21, 2021

View reviewed changes

Comment thread astropy/units/physical.py

namurphy commented Jan 21, 2021

View reviewed changes

namurphy requested a review from mhvk January 27, 2021 17:49

mhvk reviewed Jan 31, 2021

View reviewed changes

namurphy mentioned this pull request Apr 19, 2021

Add narrative documentation for physical types #11606

Merged

namurphy mentioned this pull request Apr 21, 2021

Make PhysicalType instances support string methods with deprecation warnings #11617

Closed

nstarman mentioned this pull request Apr 25, 2021

Add dimensional analysis to Units section astropy/astropy-workshop#180

Closed

nstarman pushed a commit to nstarman/astropy that referenced this pull request Apr 26, 2021

Change changelog to town crier format for astropy#11204

559f527

pllim mentioned this pull request Apr 26, 2021

Update physical type implementation #11625

Merged

4 tasks

namurphy mentioned this pull request May 3, 2021

Prevent recursion error and allow PhysicalType objects to be pickled #11686

Merged

duncanmmacleod added a commit to duncanmmacleod/gwpy that referenced this pull request Jun 21, 2021

gwpy.detector: fix enable for imperial units

c438d21

astropy/astropy#11204 removed `imperial` from being imported into `astropy.units` namespace

duncanmmacleod mentioned this pull request Jun 21, 2021

Fix enable for imperial units gwpy/gwpy#1345

Merged

duncanmmacleod added a commit to duncanmmacleod/gwpy that referenced this pull request Jun 21, 2021

gwpy.detector: fix enable for imperial units

86cb08a

astropy/astropy#11204 removed `imperial` from being imported into `astropy.units` namespace

dhomeier mentioned this pull request Jun 23, 2021

Handle new PhysicalType class in spectrum_from_column_mapping astropy/specutils#833

Merged

erwanp mentioned this pull request Jul 27, 2021

cannot import name 'imperials' from 'astropy.units' #11975

Closed

pllim reviewed Jul 27, 2021

View reviewed changes

pllim mentioned this pull request Jul 27, 2021

BUG: Fix imperials import from astropy.units #11977

Merged

9 tasks

eerovaher mentioned this pull request Oct 20, 2023

Remove deprecated functionality from units #15514

Merged

1 task

neutrinoceros mentioned this pull request Mar 28, 2024

PERF: quickly resolve inequalities for Quantities with different physical types #16248

Closed

1 task

namurphy mentioned this pull request Feb 11, 2025

Allow PhysicalType(dimension-string) to return the appropriate physical type? #17765

Open

namurphy mentioned this pull request Mar 14, 2025

Simplify PhysicalType #17884

Merged

1 task

namurphy mentioned this pull request Mar 31, 2025

Simplify PhysicalType arithmetic implementation #17962

Merged

1 task


		physical_type_id = unit._get_physical_type_id()

		known_unit = physical_type_id in _physical_unit_mapping



		def test_multiple_same_physical_type_names():
		"""Test that def_physical_type does not define a """

		assert obscure_unit.physical_type == "unknown"


		def _undef_physical_type(unit):

Uh oh!

Uh oh!

Conversation

namurphy commented Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pep8speaks commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated at 2021-04-16 18:54:54 UTC

Uh oh!

mhvk commented Jan 6, 2021

Uh oh!

namurphy commented Jan 6, 2021

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhvk commented Jan 7, 2021

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhvk commented Jan 11, 2021

Uh oh!

namurphy commented Jan 15, 2021

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

namurphy commented Jan 4, 2021 •

edited

Loading

pep8speaks commented Jan 6, 2021 •

edited

Loading

maxnoe commented Jan 19, 2021 •

edited

Loading