Some variables have different names but represent the same constituent (e.g., "Total Phosphorus, mixed forms" and "Phosphorus"). Add characteristicNames that can be converted in the same way as an existing. May require:

• re-structuring of domains.out_col_lookup()
• filtering of results to limit qualifiers to those that are the same
• further development of domains.accepted_methods()

Value criteria (not NAN) or metadata may be able to help identify a better/worse candidate between two duplicates with the same location/time etc. (e.g., replicates?)

In readme column names in Source column are separated by comma, in the rst file for docs these should be getting their own line instead. They don't have their own lines in the rst and the two are not consistent.

Currently one standard unit registry is used by the conversion decorators. This needs to either be passed in (potentially by another decorator?) or the u_reg needs to have all additions for all expected units. Currently just has 'TODO: find more elegant way to do this with all definitions'

Error from Cristina Mullins on wrangle.collapse_results(). Error from pint:

OffsetUnitCalculusError: Ambiguous operation with offset unit (degree_Celsius). See https://pint.readthedocs.io/en/latest/nonmult.html for guidance.

Suspect this occurs when multiple rows with the same index are averaged (adding temperatures can cause different results depending on their units) this is negated when it's an average but need to handle with care.

Warning message regarding array interface being deprecated and dropped in Shapely 2.0 and converting to numpy array when running location.harmonize_locations()

The goal is to accept a dict of 'accepted' methods. Currently there is a static dict of accepted methods for select characteristicNames. These can also be read from domains and/or NEMI. Some of these methods may also help define e.g., sample fraction, when unreported.

clean.wet_dry_checks() uses a combination of column values to identify questionable field values. It was constructed to capture a suspiscious dry sediment measure with water as it's 'ActivityMediaName'. Given a dict of columns and criteria it could be used to mask and then update to a given value.

wet_dry_drop() drops rows based on a filter. It too could be generalized, or updated to instead add a qa_flag for later removal (what everything else has transitioned to).

e.g., mg/l vs mg/L is handled fine by pint, but any pre-processing that updates unit-string to fix it before pint works with it have to recognize/fix both. Planned solution is either (A) upper()/lower() and make sure it doesn't change the meaning to pint, OR (B) use upper lower to build out the dict/list (not preferable because string may be longer than char that is problematic and could require many combinations). TADA uses all caps for this and other fields.

Daylight savings runs March to November, flagging time zones that suggest daylight savings outside this period.

when testing the examples in the harmonize module there is inconsistency in the order of rows:

harmonize.convert_unit_series(quantity_series, unit_series, units = 'mg/l')

and the order of columns being added:

df_result_all

Suspect the issue may be due to a non-ordered dictionary being used somewhere it shouldn't be.

List to track progress on reviewer comments. Will add/edit as we go.

• nitpick (domain.py): there is no need for a raw string for TADA_DATA_URL
• discussion (convert.py): About the TODO - both points of view (regrouping constants in a single place, or having them defined near their place of use to avoid jumping around the code base) are valid. I am usually in favor of the former.
• suggestion (domain.py): In harmonize_TADA_dict, we could use a groupby operation to avoid looping through the dataframe using python. TOCHECK
• suggestion (domain.py): replace x in list(set(pandas_series)) pattern w/ .unique method
• suggestion (domain.py, basic.py): replace one dict functions (e.g., out_col_lookup) w/ module-level dicts. Move their docstrings (e.g., sources) to module docstring.
• suggestion (convert.py): We could add "references" sections in the docstrings so that the sources are present in the website and not only in the source code.
• suggestion (basis.py, general): use pandas' methods, e.g., assign w/ np.where() for masked replace. Do compare on timeit, consistency and any other pros/cons.
• todo (init.py): importlib.metadata was added in python 3.8, which is the minimal version supported by the package according to its pyproject.toml. The try .. except block should not be needed, even more so considering that importlib_metadata is not listed in the project requirements.
• todo (basis.py): group the conditions branches in update_result_basis
• todo (contributing.rst): Add a section describing how to setup development environment (e.g. installing the test and docs dependencies).
• issue (domain.py): list sub-dependencies (e.g., requests)
• issue (domain.py): specify exception expected in re_case.

Note: several items not on the list have been or are being resolved.

see geopandas documentation for an example of this

Within harmonize.WQCharData.check_units(), the try except catches pint.UndefinedUnitError, but pint has to be able to evaluate it to get to that point. e.g., a special character unit like '#' of '#/100mL' first throws an AttributeError: 'NoneType' object has no attribute 'evaluate'. commit a18a70d added a built in fix for now but ideally need to catch/handle that error better.

Currently this is a hard-written constant in convert. If passed in as an optional variable (default dict moved to domains?) the user could specify changes.

Tests currently run locally using pytest

TADA flags location lat/lon that have few decimal places. This is important before a transform because transform may result in precision that isn't actually there (i.e. adds decimal places).

Update toml to work w/ pypi and add info for pip installing to readme/docs.

BASE_URL = 'https://cdx.epa.gov/wqx/download/DomainValues/' contains several zipped csv files to read to dict rather than having them static/hard coded.

Create first release when first release is ready

Look into using framework and gh pages

Update require at least v1.0 and leverage new functionality (e.g., replace wrangle.what_activities).

Specifically the error is:
ValueError: cannot reindex on an axis with duplicate labels

And comes up for GOM Phosphorus in _coerce_measure():
df_out[self.out_col] = meas_s

It occurs where geopandas is trying to re-index, this geopandas version has a relevant deprecation warning:
~lib\site-packages\geopandas\geodataframe.py:1443: FutureWarning: reindexing with a non-unique Index is deprecated and will raise in a future version.

When downloading large datasets it may be useful to update only what changes from a prior download. Period of record service should be part of summary webservices (not from profile). This may be better as part of dataRetrieval (at least on top, may already be in there).

This should be resolved in docs branch now - may have to update tests to get passing

Sample frames are now being published as FeatureService, can these be easily leveraged by demos instead of having a local copy in tests? Could a larger test be run with actions to ensure there is no data in these that would break? (to run periodically rather than on PRs)

wrangle.add_detection() not implemented in harmonize.harmonize_generic yet

Each of these requirements is pinned:

• pandas<2.0
• geopandas<0.13
• dataretrieval<1.0.5
  As they each have changes that would break this package.