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

Adds properties on edges #306

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Adds properties on edges #306

wants to merge 2 commits into from

Conversation

andrejtonev
Copy link

@andrejtonev andrejtonev commented Feb 28, 2024

one_to_many: passing field parameters (which already existed but was not used) many_to_many: changed from parameters to column_names_mapping
using the mapping and columns to define properties
ignoring from to keys when adding properties

Description

Responding to user-made issue. Requesting the ability to define properties on edges.
Something similar was already present, but was not used and needed to be expanded to support mappings.
Mappings are only supported for many_to_many
one_to_many only supports predefined and fixed parameters defined via the configuration file.

Pull request type

Please delete options that are not relevant.

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring with functional or API changes
  • Refactoring without functional or API changes
  • Build or packaging related changes
  • Documentation content changes
  • Other (please describe):

Related issues

Delete section if this PR doesn't resolve any issues.

Closes #301

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

######################################

Reviewer checklist (the reviewer checks this part)

  • Core feature implementation
  • Tests
  • Code documentation
  • Documentation on gqlalchemy/docs

######################################

Copy link
Contributor

@antepusic antepusic left a comment

Choose a reason for hiding this comment

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

The code looks fine. Can you add a test to serve as an example and verify that this feature works?

@andrejtonev andrejtonev changed the base branch from main to develop February 29, 2024 09:32
@andrejtonev
Copy link
Author

@antepusic I changed the code a bit. Had some CI problems, not sure why it didn't like the first version...

@andrejtonev andrejtonev changed the base branch from develop to main February 29, 2024 13:45
one_to_many: passing field parameters (which already existed but was not used)
many_to_many: changed from parameters to column_names_mapping
	      using the mapping and columns to define properties
	      ignoring from to keys when adding properties
@andrejtonev andrejtonev changed the base branch from main to develop February 29, 2024 14:16
@andrejtonev andrejtonev added Docs needed Docs needed feature feature labels Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs needed Docs needed feature feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Please allow add edge properties in Loaders
3 participants