Remove get_aiida_class#2309
Merged
Merged
Conversation
muhrin
force-pushed
the
issue_2237_rebased2
branch
from
88547ac to
71ce801
Compare
sphuber
force-pushed
the
issue_2237_rebased2
branch
from
71ce801 to
4ddda92
Compare
sphuber
requested changes
Dec 6, 2018
Contributor
sphuber
left a comment
sphuber
left a comment
There was a problem hiding this comment.
few places with dead code, but good otherwise
sphuber
force-pushed
the
issue_2237_rebased2
branch
from
4ddda92 to
0e74729
Compare
sphuber
previously approved these changes
Dec 7, 2018
giovannipizzi
requested changes
Dec 7, 2018
Member
There was a problem hiding this comment.
Better to replace this with self._dbmodel.dbnodes.all().iterator() for lazy loading also while looping with iter and next, see
see also here http://blog.etianen.com/blog/2013/06/08/django-querysets/
Contributor
Author
There was a problem hiding this comment.
Done, but we can't call count() on the iterator so I only use the iterator in the actually iteration code.
Member
There was a problem hiding this comment.
We should use a custom Iterator class as it is done in the Backend classes to have len pipe through the .count() method of the ORM, for efficiency
Member
There was a problem hiding this comment.
This should go back to len(group.nodes) once the suggested fix for len is in place, for efficiency
muhrin
force-pushed
the
issue_2237_rebased2
branch
3 times, most recently
from
86ef75d to
9693d45
Compare
giovannipizzi
previously approved these changes
Dec 7, 2018
Added convertors to go from a DbModel BackendEntity and from BackendEntity to OrmEntity.
Replaced all instances with either `get_backend_entity` or `get_orm_entity` (as appropriate). Also changed backend group to be independent of the ORM. Because the iterator was getting the ORM class and returning that, which required it to access up to the ORM level. Now it just returns backend entities which are in turn converted to ORM entities by the `orm.Group`. This also means you can't call `len(group.nodes)` because this is not allowed for iterators but the same can be achieved with `len(list(group.nodes))`
sphuber
force-pushed
the
issue_2237_rebased2
branch
from
61709bb to
1a9c869
Compare
sphuber
approved these changes
Dec 7, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2237
This is part of the procedure that @szoupanos is leading to convert to automatically generated dummy models for the
QueryBuilderReplaced all instances with either get_backend_entity or get_orm_entity
(as appropriate).
Also changed backend group to be independent of the ORM. Because the
iterator was getting the orm class and returning that (which required it
to access up to the ORM level). Now it just returns backend entities
which are in turn converted to ORM entities by the orm.Group. This also
means you can't call len(group.nodes) because this is not allowed for
iterators but the same can be achieved with
len(list(group.nodes)).Removal of
get_aiida_classfrom QueryBuilder and addition ofDbModel-to-BackendEntityandBackendEntity-to-OrmEntityconvertors.