-
Notifications
You must be signed in to change notification settings - Fork 279
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
TYP: add static typing to Dataset._get_field_info_helper #4229
Merged
matthewturk
merged 18 commits into
yt-project:main
from
neutrinoceros:exp_static_typing_get_field_info_helper
Dec 30, 2022
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
189c7d1
TYP: narrow down type of Dataset.particle_types
neutrinoceros 93508b3
TYP: add static typing to Dataset._get_field_info_helper
neutrinoceros e0dc8bc
RFC: internal var renaming
neutrinoceros 71f5754
RFC: cleanup calls to _get_field_info using the 2 args signature
neutrinoceros ccd65c5
RFC: drop second positional argument in Dataset._get_field_info
neutrinoceros 4764124
TYP: reduce runtime type flexibility in private routine Dataset._get_…
neutrinoceros e84267a
RFC: drop unused caching from Dataset._get_field_info (Dataset._last_…
neutrinoceros 0e4b2a5
TYP: improve exception specificity in Dataset._get_field_info
neutrinoceros 8f8875b
RFC: simplify one-liner
neutrinoceros 5c950af
MNT: adjust YTFieldNotFound error message to evolutions in Dataset._g…
neutrinoceros d7b5391
RFC: simplify unused branch
neutrinoceros ade1c28
MNT: cleanup unused private method
neutrinoceros 85759e5
RFC: simplify internal logic in Dataset._get_field_info_helper
neutrinoceros b7c622c
RFC: fix duck typing in Dataset._get_field_info
neutrinoceros 35e836f
TYP: use yt._typing explicit type annotations more extensively
neutrinoceros f9dc662
RFC: avoid unpacking field tuples only to reassemble in the same scope
neutrinoceros 43c1004
TYP: add type hints to YTDataContainer._first_matching_field
neutrinoceros 183425c
TST: drop a couple badly defined tests
neutrinoceros File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,26 +2,25 @@ | |
import weakref | ||
from collections import defaultdict | ||
from contextlib import contextmanager | ||
from typing import List, Tuple | ||
from typing import TYPE_CHECKING, List, Tuple | ||
|
||
import numpy as np | ||
|
||
from yt._maintenance.deprecation import issue_deprecation_warning | ||
from yt._typing import AnyFieldKey | ||
from yt._typing import AnyFieldKey, FieldKey, FieldName | ||
from yt.config import ytcfg | ||
from yt.data_objects.field_data import YTFieldData | ||
from yt.data_objects.profiles import create_profile | ||
from yt.fields.field_exceptions import NeedsGridType | ||
from yt.frontends.ytdata.utilities import save_as_dataset | ||
from yt.funcs import get_output_filename, is_sequence, iter_fields, mylog | ||
from yt.funcs import get_output_filename, iter_fields, mylog | ||
from yt.units._numpy_wrapper_functions import uconcatenate | ||
from yt.units.yt_array import YTArray, YTQuantity | ||
from yt.utilities.amr_kdtree.api import AMRKDTree | ||
from yt.utilities.exceptions import ( | ||
YTCouldNotGenerateField, | ||
YTException, | ||
YTFieldNotFound, | ||
YTFieldNotParseable, | ||
YTFieldTypeNotFound, | ||
YTNonIndexedDataContainer, | ||
YTSpatialFieldUnitError, | ||
|
@@ -30,6 +29,9 @@ | |
from yt.utilities.on_demand_imports import _firefly as firefly | ||
from yt.utilities.parameter_file_storage import ParameterFileStore | ||
|
||
if TYPE_CHECKING: | ||
from yt.data_objects.static_output import Dataset | ||
|
||
|
||
def sanitize_weight_field(ds, field, weight): | ||
field_object = ds._get_field_info(field) | ||
|
@@ -85,6 +87,8 @@ def __init__(self, ds, field_parameters): | |
# Dataset._add_object_class but it can also be passed as a parameter to the | ||
# constructor, in which case it will override the default. | ||
# This code ensures it is never not set. | ||
|
||
self.ds: "Dataset" | ||
if ds is not None: | ||
self.ds = ds | ||
else: | ||
|
@@ -162,7 +166,7 @@ def apply_units(self, arr, units): | |
except AttributeError: | ||
return self.ds.arr(arr, units=units) | ||
|
||
def _first_matching_field(self, field): | ||
def _first_matching_field(self, field: FieldName) -> FieldKey: | ||
for ftype, fname in self.ds.derived_field_list: | ||
if fname == field: | ||
return (ftype, fname) | ||
|
@@ -270,10 +274,7 @@ def __getitem__(self, key): | |
try: | ||
rv = self.field_data[f] | ||
except KeyError: | ||
if isinstance(f, tuple): | ||
fi = self.ds._get_field_info(*f) | ||
elif isinstance(f, bytes): | ||
fi = self.ds._get_field_info("unknown", f) | ||
Comment on lines
-275
to
-276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is interesting. any idea what the history of using a byte string here is? @matthewturk? |
||
fi = self.ds._get_field_info(f) | ||
rv = self.ds.arr(self.field_data[key], fi.units) | ||
return rv | ||
|
||
|
@@ -296,7 +297,7 @@ def __delitem__(self, key): | |
|
||
def _generate_field(self, field): | ||
ftype, fname = field | ||
finfo = self.ds._get_field_info(*field) | ||
finfo = self.ds._get_field_info(field) | ||
with self._field_type_state(ftype, finfo): | ||
if fname in self._container_fields: | ||
tr = self._generate_container_field(field) | ||
|
@@ -310,8 +311,7 @@ def _generate_field(self, field): | |
|
||
def _generate_fluid_field(self, field): | ||
# First we check the validator | ||
ftype, fname = field | ||
finfo = self.ds._get_field_info(ftype, fname) | ||
finfo = self.ds._get_field_info(field) | ||
if self._current_chunk is None or self._current_chunk.chunk_type != "spatial": | ||
gen_obj = self | ||
else: | ||
|
@@ -326,7 +326,7 @@ def _generate_fluid_field(self, field): | |
return rv | ||
|
||
def _generate_spatial_fluid(self, field, ngz): | ||
finfo = self.ds._get_field_info(*field) | ||
finfo = self.ds._get_field_info(field) | ||
if finfo.units is None: | ||
raise YTSpatialFieldUnitError(field) | ||
units = finfo.units | ||
|
@@ -383,7 +383,7 @@ def _generate_particle_field(self, field): | |
else: | ||
gen_obj = self._current_chunk.objs[0] | ||
try: | ||
finfo = self.ds._get_field_info(*field) | ||
finfo = self.ds._get_field_info(field) | ||
finfo.check_available(gen_obj) | ||
except NeedsGridType as ngt_exception: | ||
if ngt_exception.ghost_zones != 0: | ||
|
@@ -407,7 +407,7 @@ def _generate_particle_field(self, field): | |
ind += data.size | ||
else: | ||
with self._field_type_state(ftype, finfo, gen_obj): | ||
rv = self.ds._get_field_info(*field)(gen_obj) | ||
rv = self.ds._get_field_info(field)(gen_obj) | ||
return rv | ||
|
||
def _count_particles(self, ftype): | ||
|
@@ -1487,49 +1487,6 @@ def _field_type_state(self, ftype, finfo, obj=None): | |
obj._current_particle_type = old_particle_type | ||
obj._current_fluid_type = old_fluid_type | ||
|
||
def _tupleize_field(self, field): | ||
|
||
try: | ||
ftype, fname = field.name | ||
return ftype, fname | ||
except AttributeError: | ||
pass | ||
|
||
if is_sequence(field) and not isinstance(field, str): | ||
try: | ||
ftype, fname = field | ||
if not all(isinstance(_, str) for _ in field): | ||
raise TypeError | ||
return ftype, fname | ||
except TypeError as e: | ||
raise YTFieldNotParseable(field) from e | ||
except ValueError: | ||
pass | ||
|
||
try: | ||
fname = field | ||
finfo = self.ds._get_field_info(field) | ||
if finfo.sampling_type == "particle": | ||
ftype = self._current_particle_type | ||
if hasattr(self.ds, "_sph_ptypes"): | ||
ptypes = self.ds._sph_ptypes | ||
if finfo.name[0] in ptypes: | ||
ftype = finfo.name[0] | ||
elif finfo.is_alias and finfo.alias_name[0] in ptypes: | ||
ftype = self._current_fluid_type | ||
else: | ||
ftype = self._current_fluid_type | ||
if (ftype, fname) not in self.ds.field_info: | ||
ftype = self.ds._last_freq[0] | ||
return ftype, fname | ||
except YTFieldNotFound: | ||
pass | ||
|
||
if isinstance(field, str): | ||
return "unknown", field | ||
|
||
raise YTFieldNotParseable(field) | ||
|
||
def _determine_fields(self, fields): | ||
if str(fields) in self.ds._determined_fields: | ||
return self.ds._determined_fields[str(fields)] | ||
|
@@ -1539,9 +1496,8 @@ def _determine_fields(self, fields): | |
explicit_fields.append(field) | ||
continue | ||
|
||
ftype, fname = self._tupleize_field(field) | ||
finfo = self.ds._get_field_info(ftype, fname) | ||
|
||
finfo = self.ds._get_field_info(field) | ||
ftype, fname = finfo.name | ||
# really ugly check to ensure that this field really does exist somewhere, | ||
# in some naming convention, before returning it as a possible field type | ||
if ( | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, this is because it's always a tuple now right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More exactly, it's because we don't have a second positional argument anymore. Passing a full tuple as the first argument already worked on main, I just made it the only way to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does this line use
base_ds._get_field_info(f)
at all instead of ofbase_ds.field_info[f].units
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea, probably no good reason, but this feels just slightly out of scope here. Do you mind if I leave the line as is ?