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

Bump depedencies #263

Merged
merged 6 commits into from
Nov 14, 2024
Merged

Bump depedencies #263

merged 6 commits into from
Nov 14, 2024

Conversation

ndaelman-hu
Copy link
Contributor

No description provided.

@ndaelman-hu ndaelman-hu added the feature / enhancement New feature or request label Nov 13, 2024
@ndaelman-hu ndaelman-hu requested a review from ladinesa November 13, 2024 15:19
@ndaelman-hu ndaelman-hu self-assigned this Nov 13, 2024
@ndaelman-hu
Copy link
Contributor Author

Updated the [dev] dependencies to match nomad-lab.

@ladinesa
Copy link
Collaborator

can you perhaps also fix the error associated with the removal of metainfo.Environment

@ndaelman-hu
Copy link
Contributor Author

can you perhaps also fix the error associated with the removal of metainfo.Environment

But this is on the nomad-lab side. I don't mind removing it, as long as Markus and Theodore agree.
Moreover, is there a certain protocol for a deprecation time period for people to adapt or how to announce removal?

@ndaelman-hu ndaelman-hu requested a review from blueraft November 13, 2024 15:39
@ladinesa
Copy link
Collaborator

ladinesa commented Nov 13, 2024

can you perhaps also fix the error associated with the removal of metainfo.Environment

But this is on the nomad-lab side. I don't mind removing it, as long as Markus and Theodore agree. Moreover, is there a certain protocol for a deprecation time period for people to adapt or how to announce removal?

no i mean if there is still some lingering usage of Environment in the parsers, I remember removing them a few months back. I thought this is the reason for the failure in the tests, but it seems not

@ndaelman-hu
Copy link
Contributor Author

can you perhaps also fix the error associated with the removal of metainfo.Environment

But this is on the nomad-lab side. I don't mind removing it, as long as Markus and Theodore agree. Moreover, is there a certain protocol for a deprecation time period for people to adapt or how to announce removal?

no i mean if there is still some lingering usage of Environment in the parsers, I remember removing them a few months back. I thought this is the reason for the failure in the tests, but it seems not

No, the reason for the failure is the magres parser. (Btw, we should check on which schema the standalone plugin depends.)
There seems to be mention of Environment in the cp2k parser. Will look into it.

Copy link
Collaborator

@blueraft blueraft left a comment

Choose a reason for hiding this comment

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

Thanks!

@coveralls
Copy link

coveralls commented Nov 14, 2024

Pull Request Test Coverage Report for Build 11839729046

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 93.067%

Totals Coverage Status
Change from base Build 11727381091: 0.005%
Covered Lines: 35815
Relevant Lines: 38483

💛 - Coveralls

@ndaelman-hu
Copy link
Contributor Author

@ladinesa Should I also open an issue for deprecating

# For automatic workflows
                from nomad.search import search
                from nomad.app.v1.models import MetadataRequired

That would remove the nomad-lab[infrastructure] dependency

@ladinesa
Copy link
Collaborator

ladinesa commented Nov 14, 2024

@ladinesa Should I also open an issue for deprecating

# For automatic workflows
                from nomad.search import search
                from nomad.app.v1.models import MetadataRequired

That would remove the nomad-lab[infrastructure] dependency

i added this in todo, we can adapt my implementation in magres parser where i did not use search

@JosePizarro3
Copy link
Collaborator

i added this in todo, we can adapt my implementation in magres parser where i did not use search

please, note Sathya is developing the parser in its own repo and asked to disconnect the parser in here

@ladinesa
Copy link
Collaborator

i added this in todo, we can adapt my implementation in magres parser where i did not use search

please, note Sathya is developing the parser in its own repo and asked to disconnect the parser in here

we should discuss how to handle external developments, since imo we should have full control over parsers that we interface with nomad

@ndaelman-hu
Copy link
Contributor Author

i added this in todo, we can adapt my implementation in magres parser where i did not use search

please, note Sathya is developing the parser in its own repo and asked to disconnect the parser in here

we should discuss how to handle external developments, since imo we should have full control over parsers that we interface with nomad

This is a separate discussion. This PR just updates some dependencies.

Sathya just has to properly set up his nomad.yaml, that's all. We are NOT going to start stripping off parsers, just because somebody made their own. I agree that this is an example of a counter-argument against grouping parsers into a single repo, though.

@ndaelman-hu ndaelman-hu merged commit 84fed5d into develop Nov 14, 2024
4 checks passed
@blueraft blueraft deleted the bump_depedencies branch November 15, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants