search

aiidalab / aiidalab-widgets-base

Reusable widgets for AiiDAlab applications
MIT License
6 stars 17 forks source link

updating spglib to 2.0.2 #487

Closed AndresOrtegaGuerrero closed 1 year ago

AndresOrtegaGuerrero commented 1 year ago

This PR address #486

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (8fd20d5) 79.45% compared to head (a87f77d) 79.45%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #487 +/- ## ======================================= Coverage 79.45% 79.45% ======================================= Files 27 27 Lines 3728 3728 ======================================= Hits 2962 2962 Misses 766 766 ``` | Flag | Coverage Δ | | |---|---|---| | python-3.10 | `79.45% <ø> (ø)` | | | python-3.8 | `79.48% <ø> (ø)` | | | python-3.9 | `79.48% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

AndresOrtegaGuerrero commented 1 year ago

@danielhollas Hi! , The test I did it was to open the viewer and just check with the examples (structures i uploaded)

danielhollas commented 1 year ago

Just a question, does this mean that when this is merged and released, that the new AWB version will be incompatible with old QeApp versions?

If that is the case, I would suggest to relax the constraint here and allow both old and new spglib versions here, since it does not seem to affect the AWB code anyway.

AndresOrtegaGuerrero commented 1 year ago

@danielhollas I should do this by just spglib >=1.14 ?

danielhollas commented 1 year ago

I would do spglib >=1.14,<3 (similarly how we handle aiida-core)

yakutovicha commented 1 year ago

Just a question, does this mean that when this is merged and released, that the new AWB version will be incompatible with old QeApp versions?

I am confused, why QE app came into the picture? I checked - the QE app doesn't depend on spglib.

danielhollas commented 1 year ago

@yakutovicha presumably it depends on spglib transitively via pymatgen, as noted in #486. I think in general we want to be less restrictive in dependencies if we can, no?