Skip to content
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

Hotfix missing aiida 2.0 compatibility update #836

Merged
merged 7 commits into from
Sep 5, 2022

Conversation

ramirezfranciscof
Copy link
Member

@ramirezfranciscof ramirezfranciscof commented Aug 29, 2022

Close #835

I can contribute this but I can't do a bugfix release and I think we might want this for the upcoming tutorial (although perhaps it is better to wait until all the sections are reviewed and so we know if there is any other fix needed here).

@ramirezfranciscof
Copy link
Member Author

(oopsie I need to update my base)

@ramirezfranciscof
Copy link
Member Author

Ok, this is ready I think

@sphuber
Copy link
Contributor

sphuber commented Aug 29, 2022

Thanks @ramirezfranciscof . Guess we forgot to enable the deprecation warnings when we released 4.0, which was done in a bit of a rush. We need to address all of them instead of just these three.

@ramirezfranciscof
Copy link
Member Author

All warnings seem to have been addressed @sphuber (also tagging @mbercx now, didn't do it at first because it was a smaller corrections and thought we would just merge it yesterday).

There are 4 remaining but that seem to come from AiiDA core, so I think not much to do here.

=============================== warnings summary ===============================
tests/calculations/test_autoinvalidate_cache.py::test_exit_code_invalidates_cache[quantumespresso.cp]
  /opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/aiida/manage/tests/main.py:125: AiidaDeprecationWarning: reset_db() is deprecated, use clear_profile() instead (this will be removed in v3)
    warn_deprecation('reset_db() is deprecated, use clear_profile() instead', version=3)

tests/calculations/test_autoinvalidate_cache.py::test_exit_code_invalidates_cache[quantumespresso.xspectra]
  /opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/aiida/plugins/entry_point.py:307: AiidaDeprecationWarning: The entry point `array.kpoints` is deprecated. Please replace it with `core.array.kpoints`. (this will be removed in v3)
    warn_deprecation(

(...)
  /opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/aiida/orm/entities.py:174: AiidaDeprecationWarning: This property is deprecated, use `.collection` instead. (this will be removed in v3)
    warn_deprecation('This property is deprecated, use `.collection` instead.', version=3, stacklevel=2)

tests/parsers/test_neb.py::test_neb_default
tests/parsers/test_neb.py::test_neb_all_iterations
  /opt/hostedtoolcache/Python/3.9.13/x64/lib/python3.9/site-packages/aiida/orm/utils/managers.py:101: AiidaDeprecationWarning: dereferencing nodes with links containing double underscores is deprecated, simply replace the double underscores with a single dot instead. For example: 
  `self.inputs.some__label` can be written as `self.inputs.some.label` instead.
   (this will be removed in v3)
    warn_deprecation(

-- Docs: https://docs.pytest.org/en/stable/warnings.html
=========== 253 passed, 1 skipped, 187 warnings in 113.78s (0:01:53) ===========

@sphuber
Copy link
Contributor

sphuber commented Aug 30, 2022

I think all of these warnings should actually still be changed.

  • The first one, reset_db, is probably being used in our conftest.py somewhere. Change it to clear_profile
  • There is an instance of array.kpoints -> core.array.kpoints
  • There is an instance of Entity.collection which should be Entity.objects
  • Search for a label with double underscore __, as said in the warning that should be replaced with a .. Or did you look for those and not find anything?

@ramirezfranciscof
Copy link
Member Author

Ah, wait, now reading closely I see these don't seem to be from aiida-core using its own deprecated functions in calls to non-deprecated ones, but still from aiida-quantumespresso using deprecated functions. Then why is this not showing me the lines of aiida-quantumespresso that call the deprecated functions like the other ones did?

I'll try to check this on Friday.

@sphuber
Copy link
Contributor

sphuber commented Sep 5, 2022

@ramirezfranciscof can you still have a look at the final warnings so we can merge this?

@ramirezfranciscof
Copy link
Member Author

ramirezfranciscof commented Sep 5, 2022

@sphuber yes, sorry, all warning messages have been taken care of now.

I will say that I think the ones from the last batch would merit the consideration of why there were not being shown with a full stack trace and if there is some way to fix them. Moreover, some of the warning messages could be improved: This property is deprecated, use .collection instead doesn't explicitly mention that the property is .objects, I had to go into aiida/orm/entities.py to figure it out.

@sphuber
Copy link
Contributor

sphuber commented Sep 5, 2022

I will say that I think the ones from the last batch would merit the consideration of why there were not being shown with a full stack trace and if there is some way to fix them. Moreover, some of the warning messages could be improved: This property is deprecated, use .collection instead doesn't explicitly mention that the property is .objects, I had to go into aiida/orm/entities.py to figure it out.

Fair enough, but that is a problem for aiida-core. Nothing we can do about it here

Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @ramirezfranciscof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing 2.0 compatibility update
2 participants