search

iiasa / climate-assessment

https://climate-assessment.readthedocs.io/en/latest
MIT License
19 stars 18 forks source link

Update pyam to >=1.7.0 #31

Closed phackstock closed 1 year ago

phackstock commented 1 year ago

Updating the pyam dependency to >=1.7.0 so that is it compatible with nomenclature-iamc.

EDIT:

jkikstra commented 1 year ago

As discussed, might be worth checking out the numpy/cicero/openscm dependency first too: https://github.com/iiasa/climate-assessment/pull/26 commit https://github.com/iiasa/climate-assessment/pull/26/commits/935f38078094abfd721a1f1bd31f35880100eb94 to check if that could be updated and maybe resolve some other dependency issues.

phackstock commented 1 year ago

Updating the dependencies to scmdata==0.15.0 (from 0.14.0) seems to break some tests @jkikstra, @znicholls do you have any idea as to why that might be the case?

jkikstra commented 1 year ago

Maybe @lewisjared will know since he did that update. Only had a minute, but seems like maybe one thing that comes back is AttributeError: 'NoneType' object has no attribute 'index'

lewisjared commented 1 year ago

scmdata pins pint<0.20 because of pint-pandas not supporting newer releases of pint. We are in the process of removing the pint-pandas dependency for that reason.

Looking at the logs, the exception is in pyam.concat no? or maybe silicone might also need an update.

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/testing.py", line 408, in invoke
    return_value = cli.main(args=args or (), prog_name=prog_name, **extra)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/cli.py", line 422, in workflow
    run_workflow(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/cli.py", line 572, in run_workflow
    assessable = _harmonize_and_infill(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/cli.py", line 346, in _harmonize_and_infill
    assessable = harmonization_and_infilling(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/harmonization_and_infilling.py", line 85, in harmonization_and_infilling
    infilled, co2_infill_db, _ = run_infilling(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/infilling/__init__.py", line 298, in run_infilling
    infilled_variables = _infill_variables(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/infilling/__init__.py", line 390, in _infill_variables
    infilled_variables = infill_all_required_variables(
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/silicone/multiple_infillers/infill_all_required_emissions_for_openscm.py", line 215, in infill_all_required_variables
    to_fill = _perform_crunch_and_check(
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/silicone/multiple_infillers/infill_all_required_emissions_for_openscm.py", line 292, in _perform_crunch_and_check
    infilled = _infill_variable(cruncher, req_var, leaders, to_fill, **kwargs)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/silicone/multiple_infillers/infill_all_required_emissions_for_openscm.py", line 369, in _infill_variable
    infilled = filler(to_fill_var)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/silicone/database_crunchers/rms_closest.py", line 203, in filler
    return pyam.concat(output_ts_list)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/pyam/core.py", line 2958, in concat
    index=ret_meta.index.names,
AttributeError: 'NoneType' object has no attribute 'index'
phackstock commented 1 year ago

Updates to work with latest version of pyam. Allows nonlinear interpolation and deprecates the old way of linearly interpolating. Bugfixes to prevent inclusion of extra regions in DecomposeCollectionTimeDepRatio.

from the release of silicone 1.3.0 sound promising, I'll try that

phackstock commented 1 year ago

Looks like there's still some issue

phackstock commented 1 year ago

We did go from 18 to 9 failing tests though, so I guess we're moving in the right direction.

phackstock commented 1 year ago

Looks like we're still having some issues:

  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/testing.py", line 408, in invoke
    return_value = cli.main(args=args or (), prog_name=prog_name, **extra)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/cli.py", line 422, in workflow
    run_workflow(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/cli.py", line 599, in run_workflow
    df_climate = climate_assessment(
  File "/home/runner/work/climate-assessment/climate-assessment/src/climate_assessment/climate/__init__.py", line 190, in climate_assessment
    scenarios_to_run = pyam.concat(batch_dfs)
  File "/opt/hostedtoolcache/Python/3.9.16/x64/lib/python3.9/site-packages/pyam/core.py", line 2959, in concat
    index=ret_meta.index.names,
AttributeError: 'NoneType' object has no attribute 'index'

Do you guys have any idea?

lewisjared commented 1 year ago

Just stepped through it. It looks like a pyam bug where ret_meta in pyam.concat ends up being None because of the following block:

https://github.com/IAMconsortium/pyam/blob/449b77cb1c625b455e3675801477f19e99b30e64/pyam/core.py#L2944

    ret_data, ret_meta = [], None
    for (df, _merge_meta) in iam_dfs:
        ret_data.append(df._data)
        if _merge_meta:
            ret_meta = (
                df.meta
                if ret_meta is None
                else merge_meta(ret_meta, df.meta, ignore_meta_conflict)
            )

    # return as new IamDataFrame, this will verify integrity as part of `__init__()`
    return IamDataFrame(
        pd.concat(ret_data, verify_integrity=False),
        meta=ret_meta,
        index=ret_meta.index.names,
    )

None of the _merge_meta values were truthy so the result is ret_meta is None. I think the root cause is that all the incoming objs to be concatenated are pd.DataFrame, but I'm not familiar with pyam.concat to know if this is a regression or was never supported.

The fix could be casting each dataframe to an IamDataFrame before passing it to concat. I'm not sure why the metadata aren't merged if a dataframe is passed.

Not sure if this has been fixed in the latest pyam This was using the latest pyam release.

phackstock commented 1 year ago

Thanks @lewisjared for taking a look. Looping in @danielhuppmann for this question. Does this look like a pyam issue to you or is this something on the climate-assessment side?

danielhuppmann commented 1 year ago

I can confirm that this is a pyam bug when concatenating only pd.DataFrame objects, see https://github.com/IAMconsortium/pyam/compare/main...danielhuppmann:pyam:bug/concat_all_pd.

First question: if you know that you have all pd.DataFrame instances, why not just use 

pyam.IamDataFrame(pd.concat([<list of pd.DataFrame objects>]))
lewisjared commented 1 year ago

Thanks @danielhuppmann. Good suggestion, that looks like it fixes the issue.

phackstock commented 1 year ago

Tests are now running, thanks for the fix @danielhuppmann and thanks for the implementation @lewisjared :tada:

phackstock commented 1 year ago

Seems that we're down to one last issue. Looks like the building of the wheel for pyyaml is failing: 

...
      ext/_yaml.c:26933:25: warning: ‘PyUnicode_AsUnicode’ is deprecated [-Wdeprecated-declarations]
      26933 |                         (PyUnicode_GET_SIZE(**argname) != PyUnicode_GET_SIZE(key)) ? 1 :
            |                         ^
      In file included from /opt/hostedtoolcache/Python/3.9.16/x64/include/python3.9/unicodeobject.h:1026,
                       from /opt/hostedtoolcache/Python/3.9.16/x64/include/python3.9/Python.h:93,
                       from ext/_yaml.c:4:
      /opt/hostedtoolcache/Python/3.9.16/x64/include/python3.9/cpython/unicodeobject.h:580:45: note: declared here
        580 | Py_DEPRECATED(3.3) PyAPI_FUNC(Py_UNICODE *) PyUnicode_AsUnicode(
            |                                             ^~~~~~~~~~~~~~~~~~~
      ext/_yaml.c:26933:25: warning: ‘_PyUnicode_get_wstr_length’ is deprecated [-Wdeprecated-declarations]
      26933 |                         (PyUnicode_GET_SIZE(**argname) != PyUnicode_GET_SIZE(key)) ? 1 :
            |                         ^
      In file included from /opt/hostedtoolcache/Python/3.9.16/x64/include/python3.9/unicodeobject.h:1026,
                       from /opt/hostedtoolcache/Python/3.9.16/x64/include/python3.9/Python.h:93,
                       from ext/_yaml.c:4:
      /opt/hostedtoolcache/Python/3.9.16/x64/include/python3.9/cpython/unicodeobject.h:446:26: note: declared here
        446 | static inline Py_ssize_t _PyUnicode_get_wstr_length(PyObject *op) {
            |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~
      error: command '/usr/bin/gcc' failed with exit code 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for PyYAML
Successfully built pint black-nb
Failed to build PyYAML
ERROR: Could not build wheels for PyYAML, which is required to install pyproject.toml-based projects

details: https://github.com/iiasa/climate-assessment/actions/runs/4989551366/jobs/8933617690?pr=31

I've done a quick look around and updated the GitHub actions checkout and setup-python to their latest versions but that didn't fix it. I've also found a bunch of people having the same issue but it seems it's typically related to using outdated versions of their OS or python. Which is not the case here... Any ideas are more than welcome. Also maybe a good idea to set up a job that tries to install the package once a week or so. Just so that we're catching a failing build sooner.

lewisjared commented 1 year ago

Hmm I see it is trying a number of different botocore dependencies which in turn requires a very old version of pyyaml which doesn't have a prebuilt wheel.

This might actually be because of the major release of urllib3 which has pretty much broken the python packaging world https://github.com/boto/botocore/issues/2926. Long story short, it tries every version of botocore before trying an older version of urllib3 during the dependency resolution.

The solution could be pinning urllib3<2 to enable it to temporarily resolve the correct dependencies. Not ideal given that this isn't an immediate dependency of the project.

phackstock commented 1 year ago

Pinning awscli to the currently latest version seems to work. Not sure why it was running down the chain of all the versions anyway but I guess that's the pip default of "take the oldest compatible version".

phackstock commented 1 year ago

Tests are passing :tada:

lewisjared commented 1 year ago

What an effort @phackstock!

phackstock commented 1 year ago

Thanks a lot for your help @lewisjared :+1:

phackstock commented 1 year ago

@jkikstra and @znicholls now that it's all working, if you could review that would be great so that I can go ahead with the merge.

phackstock commented 1 year ago

Just to see if we can get two birds with one stone and close #27 as well.

phackstock commented 1 year ago

Looks like the pyam 1.8.0 dump broke four tests. Let's stick to 1.7.0 for now and maybe address #27 in a separate PR.

phackstock commented 1 year ago

@jkikstra and @znicholls sorry to ping you again, but since I tried pyam 1.8.0 and had to revert, the approval of the PR is gone. So you'd need to approve again.

jkikstra commented 1 year ago

Hey all, just to say thanks for identifying the issue and fixing it!